Skip to content

feat(slides): add +replace-slide shortcut with block-level editing#516

Open
ViperCai wants to merge 14 commits intomainfrom
feat/slide_update
Open

feat(slides): add +replace-slide shortcut with block-level editing#516
ViperCai wants to merge 14 commits intomainfrom
feat/slide_update

Conversation

@ViperCai
Copy link
Copy Markdown
Collaborator

@ViperCai ViperCai commented Apr 16, 2026

Summary

  • New +replace-slide shortcut: supports block_replace (replace a specific block) and block_insert (insert a new element), enabling element-level editing of existing slides without full-page replacement
  • CLI auto-injection: automatically injects id="<block_id>" into the replacement root element; auto-injects <content/> into <shape> elements (including self-closing form); enriches 3350001 errors with context-aware hints
  • --presentation accepts three input forms: bare token, slides URL, or wiki URL (auto-resolved via wiki.get_node)
  • lark-slides Skill docs refactored: SKILL.md trimmed to a command index; editing workflows, examples, and API references split into dedicated files
  • Doc review improvements: added mixed-action restriction callout, block_id extraction example, shortcut recommendation guidance, and other accuracy/navigation fixes

⚠️ Before Merging

Commit c0f7751 (chore(boe): pin endpoints and x-tt-env header to boe_slide_new) is a temporary BOE environment override and must be reverted before merging:

git revert c0f7751

Test Plan

  • block_replace: id auto-injected into replacement root; correct existing id unchanged, wrong id overridden
  • block_insert: self-closing <shape/> auto-expanded with <content/>; omitting insert_before_block_id appends to page end
  • Mixed block_replace + block_insert in a single --parts is rejected with a clear error prompting to split
  • --presentation with wiki URL: resolves via get_node; non-slides obj_type returns an error
  • --dry-run: shows fully injected request body without making a real API call
  • 3350001 error: hint reflects parts context (generic checklist vs. mixed-action message)
  • Non-3350001 errors: no slides-specific hint attached
  • go test ./shortcuts/slides/... passes

Summary by CodeRabbit

  • New Features

    • Added +replace-slide shortcut for block-level slide replace/insert with client-side validation (1–200 parts), automatic root ID/ injection, wiki-URL resolution, dry-run preview, and enriched error hints.
  • Documentation

    • Added comprehensive guides and examples: replace-slide reference, edit workflows, examples (insert/replace), media-upload updates, XML get/replace docs, and SKILL.md refactor.
  • Tests

    • Added extensive tests covering parsing, injection, validation, dry-run, wiki resolution, and error scenarios.

- Add +replace-slide that wraps xml_presentation.slide.replace with
  automatic id injection on block_replace replacement roots (fixes the
  undocumented 3350001 constraint) and accepts slides/wiki URLs.
- Reject str_replace at the CLI layer per product direction; keep only
  block_replace / block_insert exposed.
- Rewrite slides edit-workflow docs to pivot on +replace-slide; retire
  update-based guidance to match the upcoming server-side removal.
…0001 error hints

- Add ensureShapeHasContent() to auto-inject <content/> child on <shape>
  elements in both block_replace replacement and block_insert insertion.
  SML 2.0 schema requires <content/> on every <shape>; omitting it triggers
  3350001 (invalid param) from the backend.
- Add enrichSlidesReplaceError() to provide context-aware hints on 3350001:
  - mixed block_replace+block_insert → "split into separate calls"
  - other cases → generic checklist (block_id not found, invalid XML, etc.)
- Update skills docs to reflect the new auto-injections and 3350001 guidance.
…dry-run injection error

- Remove unused idx variable and redundant afterOpen[:] slice in
  ensureShapeHasContent; use strings.Contains for the guard check
- Surface injectBlockReplaceIDs error in DryRun instead of silently
  discarding it
…T MERGE]

Temporary override for BOE environment testing — revert before merging to main.
…gation

- SKILL.md: add quick entry for editing existing slides pointing to
  edit-workflows.md; add edit-workflows to 参考文档 table
- lark-slides-replace-slide.md: clarify id auto-inject wording; add
  self-closing <shape/> expansion note; add mixed-action / 200-item
  limit callout before parts structure; fix insert_before_block_id
  description (nil-only, not empty string)
- lark-slides-edit-workflows.md: clarify -1 revision override wording;
  sync insert_before_block_id description
- lark-slides-xml-presentation-slide-get.md: expand note 3 with
  concrete block_id format example and jq extraction command
- lark-slides-xml-presentation-slide-replace.md: add shortcut
  recommendation at top of 用途 section
- examples.md: annotate auto-inject behavior in examples 7 and 8
- add QA_feat_slide_update.md for QA handoff
@github-actions github-actions bot added the size/XL Architecture-level or global-impact change label Apr 16, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

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

Adds a new +replace-slide CLI shortcut for block-level slide replace/insert with XML post-processing, presentation (including wiki) resolution, request header/endpoint tweaks, extensive docs/examples, and comprehensive unit tests plus error-hint enrichment for backend code 3350001.

Changes

Cohort / File(s) Summary
Docs — QA & SKILL
QA_feat_slide_update.md, skills/lark-slides/SKILL.md
New QA spec and SKILL refactor to document +replace-slide behavior and shift SKILL focus to XML edit/replace workflows.
Docs — References / Examples
skills/lark-slides/references/examples.md, skills/lark-slides/references/lark-slides-*.md
Added examples, get/replace/create/delete docs updated to response envelope (data.*), new replace/get/slide workflow docs, media-upload updated to use block_insert.
Security & Endpoints
internal/cmdutil/secheader.go, internal/core/types.go
Added x-tt-env: boe_slide_new header in base headers; changed default endpoints from *.feishu.cn to *.feishu-boe.cn.
XML Helpers & Tests
shortcuts/slides/helpers.go, shortcuts/slides/helpers_test.go
Added regex-based XML helpers ensureXMLRootID and ensureShapeHasContent with thorough table-driven tests for edge cases and malformed inputs.
Shortcut Registration
shortcuts/slides/shortcuts.go
Registered new exported shortcut SlidesReplaceSlide in the shortcuts list.
Shortcut Implementation & Tests
shortcuts/slides/slides_replace_slide.go, shortcuts/slides/slides_replace_slide_test.go
Implemented +replace-slide shortcut: flags/JSON parsing, 1–200 parts limit, whitelist block_replace/block_insert (reject str_replace), inject/override replacement root id, ensure <shape> contains <content/>, wiki → presentation resolution, POST replace API call, dry-run path, 3350001 error hint enrichment, and extensive tests covering validations, dry-run, wiki resolution, and failure passthrough.

Sequence Diagram

sequenceDiagram
    participant Client
    participant CLI as CLI (Parser/Validator)
    participant Wiki as Wiki Resolver
    participant XML as XML Transformer
    participant Slides as Slides API
    participant EH as Error Enricher

    Client->>CLI: invoke +replace-slide (--presentation, --slide-id, --parts)
    CLI->>CLI: parse/validate flags and JSON parts
    CLI->>CLI: enforce parts bounds (1-200) and action whitelist
    alt presentation is wiki URL
        CLI->>Wiki: GET /open-apis/wiki/v2/spaces/get_node
        Wiki-->>CLI: xml_presentation_id
    end
    CLI->>XML: ensureXMLRootID(replacement, block_id)
    XML-->>CLI: normalized replacement (id injected/overridden)
    CLI->>XML: ensureShapeHasContent(xml)
    XML-->>CLI: xml with <content/> injected when needed
    CLI->>Slides: POST /open-apis/slides_ai/v1/xml_presentations/{id}/slide/replace (parts)
    Slides-->>CLI: success {revision_id, parts_count} or error
    alt error code 3350001
        CLI->>EH: add parts/context-aware hint
        EH-->>CLI: enriched error
        CLI-->>Client: exit with enriched hint
    else success
        CLI-->>Client: print result (xml_presentation_id, slide_id, parts_count, ...)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

domain/ccm

Suggested reviewers

  • fangshuyu-768

Poem

🐰 I nibble XML by moonlit code,
I tuck an id where roots once strode;
I stitch a snug and small,
Resolve the wiki token, one and all;
Now slides hop ready — spring to call! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 74.19% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: introducing a new +replace-slide shortcut with block-level editing capabilities for slides.
Description check ✅ Passed The description covers summary, changes, test plan, and related issues sections as required by the template. All critical information about the feature is present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/slide_update

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 and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 16, 2026

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#feat/slide_update -y -g

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (2)
shortcuts/slides/slides_replace_slide.go (1)

105-119: DryRun URL should use path segment encoding for consistency with Execute.

In Execute (line 158), the presentation ID is encoded via validate.EncodePathSegment(presentationID), but here in DryRun the ID is interpolated directly. While presentation IDs typically don't contain special characters, the dry-run output should accurately reflect what will be sent.

♻️ Suggested fix
 		dry.POST(fmt.Sprintf("/open-apis/slides_ai/v1/xml_presentations/%s/slide/replace", presentationID)).
+		// Note: In actual Execute, presentationID is encoded via validate.EncodePathSegment
 			Params(query).
 			Body(body)

Alternatively, apply the same encoding:

-	dry.POST(fmt.Sprintf("/open-apis/slides_ai/v1/xml_presentations/%s/slide/replace", presentationID)).
+	dry.POST(fmt.Sprintf("/open-apis/slides_ai/v1/xml_presentations/%s/slide/replace", validate.EncodePathSegment(presentationID))).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/slides/slides_replace_slide.go` around lines 105 - 119, The DryRun
implementation builds the POST path by interpolating presentationID directly;
make it mirror Execute by path-encoding the ID using
validate.EncodePathSegment(presentationID) before constructing the POST URL in
DryRun (the block that calls dry.POST(fmt.Sprintf(...))). Ensure presentationID
assignment/override logic (including the wiki branch) stays the same, but pass
the encoded value into the POST path and still return dry.Set("parts_count",
len(parts)).
shortcuts/slides/slides_replace_slide_test.go (1)

533-568: Test assertion doesn't match implementation's actual hint text.

The test checks that exitErr.Detail.Hint does not contain "missing <content/>", but buildSlides3350001Hint never produces that string. The actual hints are:

  • "mixed block_replace+block_insert..."
  • "common causes: (1) block_id not found..."

This assertion will always pass regardless of whether enrichment occurred, making it a weak test.

♻️ Suggested fix
-	if strings.Contains(exitErr.Detail.Hint, "missing <content/>") {
-		t.Fatalf("non-3350001 error should not get slides-specific hint, got %q", exitErr.Detail.Hint)
+	if strings.Contains(exitErr.Detail.Hint, "common causes") ||
+		strings.Contains(exitErr.Detail.Hint, "mixed block_replace") {
+		t.Fatalf("non-3350001 error should not get slides-specific hint, got %q", exitErr.Detail.Hint)
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/slides/slides_replace_slide_test.go` around lines 533 - 568, The
test TestReplaceSlideNon3350001ErrorNotEnriched currently asserts that
exitErr.Detail.Hint does not contain "missing <content/>", but
buildSlides3350001Hint never produces that text; update the assertion to check
for the actual slides-specific hints instead: ensure exitErr.Detail.Hint does
not contain the substrings that buildSlides3350001Hint can produce (e.g., "mixed
block_replace+block_insert" and "common causes:"), or assert Hint is empty/nil;
modify the check in TestReplaceSlideNon3350001ErrorNotEnriched to look for those
real substrings on exitErr.Detail.Hint (referencing exitErr.Detail.Hint,
buildSlides3350001Hint, and the test function name) so the test truly verifies
that non-3350001 errors are not enriched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/cmdutil/secheader.go`:
- Around line 24-27: The code adds a hardcoded BOE test header (HeaderTtEnv /
TtEnvValue) and injects it in BaseSecurityHeaders(), forcing all traffic into
the BOE environment; remove this temporary override by deleting TtEnvValue (or
set it nil) and stop adding HeaderTtEnv in BaseSecurityHeaders() by default, or
gate its insertion behind an explicit opt-in (e.g., an environment variable or
feature flag) so the header is only added when that flag is set; update any
tests/configs that relied on the temporary header accordingly.

In `@internal/core/types.go`:
- Around line 43-45: Revert the temporary BOE endpoint override introduced in
commit c0f7751 that changed the feishu endpoints (the Open, Accounts, MCP fields
currently set to "https://open.feishu-boe.cn", "https://accounts.feishu-boe.cn",
"https://mcp.feishu-boe.cn"); undo that commit (git revert c0f7751) or restore
the original production URLs for those fields in the same struct/constants so
production traffic is not redirected, then verify the values in the same
symbol/block are back to the expected production endpoints and run CI/tests to
confirm no regressions.

In `@QA_feat_slide_update.md`:
- Around line 133-140: Test Case 24 incorrectly expects backend error 3350001
for a mixed-action input; instead update the test to assert the CLI/client-side
validation path for mixed actions (i.e., validate that providing both
block_replace and block_insert in the same request is rejected before hitting
the backend). Modify the Case 24 assertion to check for the CLI validation error
message/status returned by the validation routine (the mixed
block_replace+block_insert check) and remove the expectation of error code
3350001.

In `@shortcuts/slides/helpers.go`:
- Around line 166-170: The xmlIdAttrRegex is too permissive and can match
attributes like data-id or xml:id; update the regex used by ensureXMLRootID
(xmlIdAttrRegex) and any other occurrences (lines ~199-209) to require an exact
attribute name "id" not preceded by name characters or ':'/ '-' (e.g., use a
negative lookbehind like (?<![A-Za-z0-9:_-])id\s*=\s*(["'])(.*?)\1) so it only
matches the standalone id attribute and also ensure the closing quote is the
same as the opening quote (use backreference \1).
- Around line 248-251: The current substring check on afterOpen using
strings.Contains(xmlFragment[m[1]:], "<content") can false-positive on tags like
<contention/>; instead perform a proper tag match (e.g. use
regexp.MustCompile(`^<content(\s|>|/)`) or search for `"<content"` followed by
whitespace, `>` or `/`) against the sliced fragment (afterOpen) before returning
xmlFragment; update the check that references afterOpen and xmlFragment to use
this stricter pattern so only actual <content> tags are detected.

In `@skills/lark-slides/references/lark-slides-edit-workflows.md`:
- Around line 9-14: The third table row currently suggests using a single
`--parts` for "换标题 + 加图", which contradicts the mixed-action restriction; update
that row to explicitly state that when performing multiple changes that mix
`block_replace` and `block_insert` you must split them into separate same-action
calls (e.g., one atomic `--parts` batch of only `block_replace` operations and a
separate batch of only `block_insert` operations), and mention that mixed
`block_replace + block_insert` must not be combined in a single `--parts`.

In `@skills/lark-slides/references/lark-slides-replace-slide.md`:
- Around line 58-59: The "限制" rule currently forbids mixed actions in one
--parts batch but the provided batch example mixes `block_replace` and
`block_insert`; either split that example into two separate CLI calls (one all
`block_replace`, one all `block_insert`) or change the rule text to explicitly
allow mixed actions and update the example to match; make the same correction
for the other occurrence where the mixed-example appears (the block around the
later example that also shows `block_replace` + `block_insert`) so text and
examples are consistent.

---

Nitpick comments:
In `@shortcuts/slides/slides_replace_slide_test.go`:
- Around line 533-568: The test TestReplaceSlideNon3350001ErrorNotEnriched
currently asserts that exitErr.Detail.Hint does not contain "missing
<content/>", but buildSlides3350001Hint never produces that text; update the
assertion to check for the actual slides-specific hints instead: ensure
exitErr.Detail.Hint does not contain the substrings that buildSlides3350001Hint
can produce (e.g., "mixed block_replace+block_insert" and "common causes:"), or
assert Hint is empty/nil; modify the check in
TestReplaceSlideNon3350001ErrorNotEnriched to look for those real substrings on
exitErr.Detail.Hint (referencing exitErr.Detail.Hint, buildSlides3350001Hint,
and the test function name) so the test truly verifies that non-3350001 errors
are not enriched.

In `@shortcuts/slides/slides_replace_slide.go`:
- Around line 105-119: The DryRun implementation builds the POST path by
interpolating presentationID directly; make it mirror Execute by path-encoding
the ID using validate.EncodePathSegment(presentationID) before constructing the
POST URL in DryRun (the block that calls dry.POST(fmt.Sprintf(...))). Ensure
presentationID assignment/override logic (including the wiki branch) stays the
same, but pass the encoded value into the POST path and still return
dry.Set("parts_count", len(parts)).
🪄 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: f856f9b4-a92d-4fdc-9f24-362a073a93f6

📥 Commits

Reviewing files that changed from the base of the PR and between 35a8288 and 0001416.

📒 Files selected for processing (15)
  • QA_feat_slide_update.md
  • internal/cmdutil/secheader.go
  • internal/core/types.go
  • shortcuts/slides/helpers.go
  • shortcuts/slides/helpers_test.go
  • shortcuts/slides/shortcuts.go
  • shortcuts/slides/slides_replace_slide.go
  • shortcuts/slides/slides_replace_slide_test.go
  • skills/lark-slides/SKILL.md
  • skills/lark-slides/references/examples.md
  • skills/lark-slides/references/lark-slides-edit-workflows.md
  • skills/lark-slides/references/lark-slides-media-upload.md
  • skills/lark-slides/references/lark-slides-replace-slide.md
  • skills/lark-slides/references/lark-slides-xml-presentation-slide-get.md
  • skills/lark-slides/references/lark-slides-xml-presentation-slide-replace.md

Comment thread internal/cmdutil/secheader.go Outdated
Comment thread internal/core/types.go Outdated
Comment thread QA_feat_slide_update.md Outdated
Comment on lines +133 to +140
### 4.6 3350001 错误提示增强

| # | 用例 | 预期 hint 关键词 |
|---|------|----------------|
| 22 | 单纯 `block_replace` 触发 3350001 | `common causes`(block_id 不存在 / 坐标越界等) |
| 23 | 单纯 `block_insert` 触发 3350001 | `common causes` |
| 24 | mixed `block_replace` + `block_insert` 触发 3350001 | `mixed block_replace+block_insert` |
| 25 | 其他错误码(非 3350001) | 不附加 slides 专属 hint |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Mixed-action QA expectation appears misaligned with the feature contract.

Case 24 currently expects backend 3350001, but the feature behavior is documented as rejecting mixed actions up front. This test should assert the CLI validation error path instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@QA_feat_slide_update.md` around lines 133 - 140, Test Case 24 incorrectly
expects backend error 3350001 for a mixed-action input; instead update the test
to assert the CLI/client-side validation path for mixed actions (i.e., validate
that providing both block_replace and block_insert in the same request is
rejected before hitting the backend). Modify the Case 24 assertion to check for
the CLI validation error message/status returned by the validation routine (the
mixed block_replace+block_insert check) and remove the expectation of error code
3350001.

Comment thread shortcuts/slides/helpers.go Outdated
Comment thread shortcuts/slides/helpers.go
Comment thread skills/lark-slides/references/lark-slides-edit-workflows.md
Comment thread skills/lark-slides/references/lark-slides-replace-slide.md Outdated
…xed-action contradictions

Code fixes:
- xmlIdAttrRegex: replace \bid with (?:^|\s)id so attributes like data-id
  and xml:id (where \b fires on '-'/':') are not accidentally matched
- ensureShapeHasContent: replace strings.Contains("<content") with a regex
  check (<content(?:\s|/|>)) to avoid false-positive on <contention/> etc.
- Add regression tests for both fixes

Doc fixes:
- lark-slides-replace-slide.md: replace mixed block_replace+block_insert batch
  example with same-action example; add note to split different actions
- lark-slides-edit-workflows.md: fix decision table (add explicit row for
  mixed-action scenario requiring two calls); fix batch example to use same
  action type only
…alse constraint

Empirically verified on BOE: a single --parts containing both block_replace
and block_insert actions succeeds (revision_id returned, all parts applied).
The prior assumption that mixing triggers 3350001 was wrong.

- Remove mixed-action branch from buildSlides3350001Hint; the function now
  always returns the generic 'common causes' checklist
- Update test: 'mixed actions gets specific hint' → 'mixed actions gets
  generic hint' (wantHint: "common causes")
- Docs: remove 'all parts must share the same action' constraint from
  replace-slide.md and edit-workflows.md; restore mixed-action batch examples
- QA Case 24: update expected hint from 'mixed block_replace+block_insert'
  to 'common causes'
…ons based on BOE testing

Empirically tested on BOE:
- revision_id: stale-but-existing revision_id is accepted (not rejected with 409);
  only non-existent revision_id (> current) returns 3350002 not found.
  Docs previously said stale revision → 400/409 conflict, which is wrong.
- failed_part_index: wrong block_id returns 3350001 invalid param (not a
  200 success with failed_part_index field). Updated error tables accordingly.

Updated files:
- lark-slides-replace-slide.md: fix --revision-id param description, fix
  error table (failed_part_index row → 3350001; 400/409 row → 3350002)
- lark-slides-edit-workflows.md: rewrite revision_id section to reflect actual
  behavior; fix batch parts description; fix error table rows
- lark-slides-xml-presentation-slide-replace.md: fix error table to match
  actual error codes (3350001 / 3350002 instead of 400 / 400/409)
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
shortcuts/slides/slides_replace_slide_test.go (1)

47-60: Consider extracting a small helper for request body decode/assert patterns.

Several tests repeat json.Unmarshal + parts shape assertions; a helper would reduce duplication and make failures more uniform.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/slides/slides_replace_slide_test.go` around lines 47 - 60, Extract
a small test helper to centralize the repeated json.Unmarshal + parts
assertions: create a function (e.g. decodePartsOrFail or assertDecodedParts)
that takes testing.T, the stub payload (stub.CapturedBody) and expected count,
performs json.Unmarshal into the same anonymous/typed struct (with Parts
[]struct{Action, BlockID, Replacement string `json:"..."`}), fails via t.Fatalf
with a consistent message on unmarshal or wrong length, and returns the parsed
parts (so tests can use the returned slice instead of repeating the decode and
len check); replace the repeated block in slides_replace_slide_test.go with a
call to this helper and use the first element (previously got := body.Parts[0])
from the returned slice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@shortcuts/slides/slides_replace_slide_test.go`:
- Around line 47-60: Extract a small test helper to centralize the repeated
json.Unmarshal + parts assertions: create a function (e.g. decodePartsOrFail or
assertDecodedParts) that takes testing.T, the stub payload (stub.CapturedBody)
and expected count, performs json.Unmarshal into the same anonymous/typed struct
(with Parts []struct{Action, BlockID, Replacement string `json:"..."`}), fails
via t.Fatalf with a consistent message on unmarshal or wrong length, and returns
the parsed parts (so tests can use the returned slice instead of repeating the
decode and len check); replace the repeated block in
slides_replace_slide_test.go with a call to this helper and use the first
element (previously got := body.Parts[0]) from the returned slice.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5166458c-439c-42dc-9cfe-a25862e0b1d3

📥 Commits

Reviewing files that changed from the base of the PR and between b742f5b and 8a34e9c.

📒 Files selected for processing (6)
  • QA_feat_slide_update.md
  • shortcuts/slides/slides_replace_slide.go
  • shortcuts/slides/slides_replace_slide_test.go
  • skills/lark-slides/references/lark-slides-edit-workflows.md
  • skills/lark-slides/references/lark-slides-replace-slide.md
  • skills/lark-slides/references/lark-slides-xml-presentation-slide-replace.md
✅ Files skipped from review due to trivial changes (2)
  • skills/lark-slides/references/lark-slides-edit-workflows.md
  • skills/lark-slides/references/lark-slides-replace-slide.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • shortcuts/slides/slides_replace_slide.go

…l_presentation.slide get

- All native API commands wrap responses in { code, data, msg } — jq paths
  need .data prefix (e.g. .data.slide.content, .data.revision_id)
- xml_presentation.slide get was missing from CLI due to stale remote_meta
  cache overriding embedded meta_data.json; embedded data has all 4 methods
  (create/delete/get/replace)
- Fix grep -oP (Linux-only) to grep -oE + sed in block_id extraction example
- Remove incorrect xmlns note from slide get doc (slide XML doesn't carry xmlns)
- Fix +media-upload jq path: .file_token → .data.file_token
…create/delete/replace docs

All three native API commands wrap responses in { code, data, msg }; update
return value JSON examples and field path descriptions accordingly.
- slide create: data.slide_id, data.revision_id
- slide delete: data.revision_id
- slide replace: data.revision_id, data.failed_part_index, data.failed_reason
- slide create/delete: add { code, data, msg } wrapper to response examples
- +replace-slide success: add { ok, data } wrapper
- +replace-slide failure: replace incorrect failed_part_index example with
  actual 3350001 error response (wrong block_id returns error, not partial success)
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@skills/lark-slides/references/lark-slides-xml-presentation-slide-replace.md`:
- Line 171: Update the Chinese wording in the table row that currently reads
"`replacement` / `insertion` 必须是合法 XML 片段,标签闭合 + 属性引号" to use the more natural
phrasing by changing "必须是合法 XML 片段" to "必须为合法的 XML 片段", i.e. make the cell read
"`replacement` / `insertion` 必须为合法的 XML 片段,标签闭合 + 属性引号"; locate the string in
the markdown table and replace it accordingly.
- Around line 116-128: The example shows mixed actions ("block_replace" +
"block_insert") in the same "parts" payload which conflicts with the PR's
shortcut behavior (the "+replace-slide" shortcut rejects mixed actions); update
the docs to avoid misleading users by either changing the example under "多条
parts 原子执行" to use only a single action (e.g., both entries as "block_replace"
or both as "block_insert"), or add a clear note under that subsection stating
that mixed actions are only supported by the low-level API and that the
"+replace-slide" shortcut requires splitting mixed operations into separate
calls (so users know to call the shortcut twice instead of sending mixed
"parts").
🪄 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: bd683048-4b49-44b8-859f-59f94f1ef022

📥 Commits

Reviewing files that changed from the base of the PR and between b786f31 and fa9c929.

📒 Files selected for processing (3)
  • 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-xml-presentation-slide-replace.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • skills/lark-slides/references/lark-slides-xml-presentation-slide-delete.md

Comment on lines +116 to +128
### 多条 parts 原子执行

```bash
lark-cli slides xml_presentation.slide replace --as user --params '{
"xml_presentation_id": "slides_example_presentation_id",
"slide_id": "slide_example_id"
}' --data '{
"parts": [
{"action":"block_replace","block_id":"bab","replacement":"<shape type=\"text\" topLeftX=\"80\" topLeftY=\"80\" width=\"800\" height=\"120\"><content textType=\"title\"><p>新标题</p></content></shape>"},
{"action":"block_insert","insertion":"<img src=\"<file_token>\" topLeftX=\"700\" topLeftY=\"400\" width=\"180\" height=\"100\"/>"}
]
}'
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Mixed-action示例与本 PR 的快捷方式限制存在歧义,建议显式拆分或加警示。

Line 116-128 给出 block_replace + block_insert 同一 parts 示例;但本 PR 明确 +replace-slide 会拒绝 mixed actions。结合 Line 7 的“优先使用 shortcut”推荐,这里会误导用户复制后在 shortcut 上失败。建议:

  1. 把该示例改成同 action;或
  2. 在该小节加一句“仅底层 API 可这样组织;+replace-slide 需拆成两次调用”。
✏️ Suggested doc patch
-### 多条 parts 原子执行
+### 多条 parts 原子执行(同类 action 示例)
@@
-  "parts": [
-    {"action":"block_replace","block_id":"bab","replacement":"<shape type=\"text\" topLeftX=\"80\" topLeftY=\"80\" width=\"800\" height=\"120\"><content textType=\"title\"><p>新标题</p></content></shape>"},
-    {"action":"block_insert","insertion":"<img src=\"<file_token>\" topLeftX=\"700\" topLeftY=\"400\" width=\"180\" height=\"100\"/>"}
-  ]
+  "parts": [
+    {"action":"block_replace","block_id":"bab","replacement":"<shape type=\"text\" topLeftX=\"80\" topLeftY=\"80\" width=\"800\" height=\"120\"><content textType=\"title\"><p>新标题</p></content></shape>"},
+    {"action":"block_replace","block_id":"bac","replacement":"<shape type=\"text\" topLeftX=\"80\" topLeftY=\"240\" width=\"800\" height=\"100\"><content><p>副标题</p></content></shape>"}
+  ]
 }'
 ## 注意事项
+8. **关于 mixed actions**:若使用 shortcut [`+replace-slide`](lark-slides-replace-slide.md),单次 `--parts` 不支持同时混用 `block_replace` 与 `block_insert`,请拆成两次调用。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-slides/references/lark-slides-xml-presentation-slide-replace.md`
around lines 116 - 128, The example shows mixed actions ("block_replace" +
"block_insert") in the same "parts" payload which conflicts with the PR's
shortcut behavior (the "+replace-slide" shortcut rejects mixed actions); update
the docs to avoid misleading users by either changing the example under "多条
parts 原子执行" to use only a single action (e.g., both entries as "block_replace"
or both as "block_insert"), or add a clear note under that subsection stating
that mixed actions are only supported by the low-level API and that the
"+replace-slide" shortcut requires splitting mixed operations into separate
calls (so users know to call the shortcut twice instead of sending mixed
"parts").

| 3350001 | `block_id` 在当前页不存在,或 XML 格式 / 结构错误 | 重新 `slide.get` 拿最新 XML,确认 `block_id` 存在;检查 `replacement` / `insertion` 是否合法 XML |
| 400 | `parts` 长度超过 200 | 拆多次调用 |
| 3350002 | `revision_id` 不存在(超过当前版本号) | 用 `-1` 或实际存在的 `revision_id` |
| 400 | XML 格式错误 | `replacement` / `insertion` 必须是合法 XML 片段,标签闭合 + 属性引号 |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Line 171 中文表述可微调为更自然用法。

必须是合法 XML 片段 建议改为 必须为合法的 XML 片段

🧰 Tools
🪛 LanguageTool

[uncategorized] ~171-~171: 您不可用“是”来表达个形容词。大部分人都平常用“很”来代替“是”。您是不是想表达"很合法"?
Context: ...ML 格式错误 | replacement / insertion 必须是合法 XML 片段,标签闭合 + 属性引号 | | 403 | 权限不足 | 需要 ...

(SHI_ADHECTIVE_ERROR)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-slides/references/lark-slides-xml-presentation-slide-replace.md`
at line 171, Update the Chinese wording in the table row that currently reads
"`replacement` / `insertion` 必须是合法 XML 片段,标签闭合 + 属性引号" to use the more natural
phrasing by changing "必须是合法 XML 片段" to "必须为合法的 XML 片段", i.e. make the cell read
"`replacement` / `insertion` 必须为合法的 XML 片段,标签闭合 + 属性引号"; locate the string in
the markdown table and replace it accordingly.

Reverts test-only changes that pointed all requests to feishu-boe.cn
and injected x-tt-env header. These must not be merged to main.
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 74.42922% with 56 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@44e7b5b). Learn more about missing BASE report.

Files with missing lines Patch % Lines
shortcuts/slides/slides_replace_slide.go 70.05% 28 Missing and 25 partials ⚠️
shortcuts/slides/helpers.go 95.12% 1 Missing and 1 partial ⚠️
shortcuts/slides/shortcuts.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #516   +/-   ##
=======================================
  Coverage        ?   59.15%           
=======================================
  Files           ?      385           
  Lines           ?    32855           
  Branches        ?        0           
=======================================
  Hits            ?    19437           
  Misses          ?    11583           
  Partials        ?     1835           

☔ View full report in Codecov by Sentry.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Architecture-level or global-impact change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant