feat(drive): extend +add-comment to support slides targets#674
Open
feat(drive): extend +add-comment to support slides targets#674
Conversation
github-actions
Bot
added
domain/ccm
|
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:
📝 WalkthroughWalkthroughAdds Feishu Slides support to the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (+add-comment)
participant Resolver as Wiki/URL Resolver
participant Validator as Validator
participant Builder as Request Builder
participant DriveAPI as Drive File API
CLI->>Resolver: resolve `--doc` (URL, token, or explicit `--type`)
Resolver-->>CLI: returns target.Kind (e.g., slides) and file token
CLI->>Validator: validate flags and `--block-id` format for slides
Validator-->>CLI: ok / error
CLI->>Builder: buildCommentCreateV2Request(fileType=slides, blockID, slideBlockType)
Builder-->>DriveAPI: POST /file/comments.create_v2 (anchor.block_id + anchor.slide_block_type)
DriveAPI-->>CLI: response (comment_id / error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@e4341773d58a7dfd9d0906db1b24b0094a551c2f🧩 Skill updatenpx skills add larksuite/cli#feat/slide_comment -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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
skills/lark-drive/references/lark-drive-add-comment.md (1)
134-148:⚠️ Potential issue | 🟡 MinorRefresh general bullets to reflect slides support.
The slides-specific block at lines 143–148 is good, but a few earlier statements are now stale and could mislead readers (including agents):
- Line 134:
--full-commentdescription says "（不适用于 sheet）" — slides also rejects--full-comment, per the validation added indrive_add_comment.go.- Line 135:
--block-idrow only documents the sheet format; consider mentioning the slides<slide-block-type>!<xml-id>format alongside.- Line 141: "传
--block-id时，shortcut 创建局部评论（划词评论）；该模式仅支持docx..." — both sheet (line 142) and slides (lines 143–148) also rely on--block-id, so the "仅支持 docx" claim contradicts the next two bullets.📝 Proposed wording refresh
-| `--full-comment` | 否 | 显式指定创建全文评论；未传 `--block-id` 时也会默认走全文评论（不适用于 sheet） | -| `--block-id` | 局部评论时必填 | 目标块 ID，可通过 `docs +fetch --api-version v2 --detail with-ids` 获取。**Sheet 评论**：格式为 `<sheetId>!<cell>`（如 `a281f9!D6`） | +| `--full-comment` | 否 | 显式指定创建全文评论；未传 `--block-id` 时也会默认走全文评论（不适用于 sheet 或 slides） | +| `--block-id` | 局部评论时必填 | 目标块 ID，可通过 `docs +fetch --api-version v2 --detail with-ids` 获取。**Sheet 评论**：格式为 `<sheetId>!<cell>`（如 `a281f9!D6`）。**Slide 评论**：格式为 `<slide-block-type>!<xml-id>`（如 `shape!bPq`） | @@ -- 传 `--block-id` 时，shortcut 创建**局部评论（划词评论）**；该模式仅支持 `docx`，以及最终可解析为 `docx` 的 wiki URL。 +- 传 `--block-id` 时，shortcut 创建**局部评论**；`docx` 模式下相当于划词评论，并支持最终可解析为 `docx` 的 wiki URL。Sheet/slides 的局部评论详见下文专门说明。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-drive/references/lark-drive-add-comment.md` around lines 134 - 148, Update the docs to reflect slides support and flag validation: change the `--full-comment` description to note it is not valid for sheet or slides, update the `--block-id` table row to mention both sheet format (`<sheetId>!<cell>`) and slides format (`<slide-block-type>!<xml-id>`), and reword the paragraph that currently says "仅支持 docx" (reference the comment about `--block-id`) so it clarifies that using `--block-id` creates a local/block comment for docx, sheets, and slides (with slides requiring the `<slide-block-type>!<xml-id>` form); ensure consistency with the validation logic implemented in drive_add_comment.go and with the Slide Parameters Mapping section.shortcuts/drive/drive_add_comment.go (1)
146-187:⚠️ Potential issue | 🔴 CriticalCritical: hard-coded
commentModeFullbreaks wiki → slides dry-run.When resolving a wiki URL in
DryRun(line 148),resolveCommentTargetis called with hard-codedcommentModeFullregardless of the user's--block-idor--full-commentflags. However,resolveCommentTargetrejectsobjType == "slides"withmode == commentModeFull(line 454), returning an error for any wiki URL that resolves to slides—even with a valid--block-id "shape!<id>".Because the error is silently swallowed (
if err == nil, line 149),resolvedKindstays"wiki"and the slides dry-run branch (line 172) is never entered. The code falls through to the doc/docx path, emitting a "create local comment" or "create full comment" description instead of "slide block comment".This breaks the intent of
TestDryRunWikiResolvesToSlides, which expects stdout to contain"slide block comment"when passing--block-id "shape!shape_2"with a wiki URL that resolves to slides.Compute the actual mode once and pass it through:
🔧 Proposed fix
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { docRef, _ := parseCommentDocRef(runtime.Str("doc"), runtime.Str("type")) replyElements, _ := parseCommentReplyElements(runtime.Str("content")) blockID := strings.TrimSpace(runtime.Str("block-id")) + selection := runtime.Str("selection-with-ellipsis") + mode := resolveCommentMode(runtime.Bool("full-comment"), selection, blockID) // For wiki URLs, resolve the actual target type via API so dry-run // matches real execution behavior instead of guessing from --block-id. resolvedKind := docRef.Kind resolvedToken := docRef.Token isWiki := false if docRef.Kind == "wiki" { isWiki = true - target, err := resolveCommentTarget(ctx, runtime, runtime.Str("doc"), commentModeFull) + target, err := resolveCommentTarget(ctx, runtime, runtime.Str("doc"), mode) if err == nil { resolvedKind = target.FileType resolvedToken = target.FileToken } } @@ // Doc/docx comment dry-run. - selection := runtime.Str("selection-with-ellipsis") - mode := resolveCommentMode(runtime.Bool("full-comment"), selection, blockID) - createPath := "/open-apis/drive/v1/files/:file_token/new_comments"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_add_comment.go` around lines 146 - 187, The dry-run wiki→slides path is broken because resolveCommentTarget is called with the hard-coded commentModeFull; compute the actual comment mode used for this invocation (based on the user's flags/--block-id/--full-comment logic already used elsewhere in this routine) into a local variable (e.g., mode) and pass that variable to resolveCommentTarget instead of commentModeFull; update the call in the wiki branch (where resolveCommentTarget is invoked) to use that computed mode so slides targets are not rejected and the slides dry-run branch can be taken (refer to resolveCommentTarget and commentModeFull to locate the change).
🧹 Nitpick comments (3)
shortcuts/drive/drive_add_comment.go (3)
688-704:parseSlidesBlockRefaccepts!inside the xml-id.
SplitN(blockID, "!", 2)keeps any additional!inside the second part, so"shape!foo!bar"parses as type=shape, id=foo!bar. PPT XML element ids in this codebase don't appear to legitimately contain!, so silently accepting it can mask user typos (e.g. accidentally pasting two anchors). Consider rejecting more than one separator.♻️ Proposed tightening
- parts := strings.SplitN(blockID, "!", 2) - if len(parts) != 2 { + parts := strings.Split(blockID, "!") + if len(parts) != 2 { return "", "", output.ErrValidation("slide --block-id must be <slide-block-type>!<xml-id> (e.g. shape!bPq), got %q", blockID) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_add_comment.go` around lines 688 - 704, The function parseSlidesBlockRef currently allows extra '!' characters inside the xml-id; update it to validate there is exactly one '!' separator by checking the count of '!' in blockID and returning the same validation error if the count is not exactly 1, then split on that single separator to populate parsedType and parsedID (still trimming and ensuring neither is empty). Keep the error messages and returned values consistent with existing output.ErrValidation usage and reference parseSlidesBlockRef, blockID, parsedType, and parsedID when making the change.
670-678: Optional: tidy the anchor mutation.The chained type assertion
body["anchor"].(map[string]interface{})["slide_block_type"] = ...works but is a bit awkward; binding the map to a local makes the intent clearer and avoids re-asserting.♻️ Proposed refactor
} else if strings.TrimSpace(blockID) != "" { - body["anchor"] = map[string]interface{}{ + anchor := map[string]interface{}{ "block_id": blockID, } - if strings.TrimSpace(slideBlockType) != "" { - body["anchor"].(map[string]interface{})["slide_block_type"] = strings.TrimSpace(slideBlockType) - } + if t := strings.TrimSpace(slideBlockType); t != "" { + anchor["slide_block_type"] = t + } + body["anchor"] = anchor }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_add_comment.go` around lines 670 - 678, The anchor map mutation in the block handling is awkwardly done via a chained type assertion on body["anchor"]; instead, after creating/assigning body["anchor"] = map[string]interface{}{ "block_id": blockID }, cast body["anchor"] to a local variable (e.g., anchor := body["anchor"].(map[string]interface{})) and then set anchor["slide_block_type"] = strings.TrimSpace(slideBlockType) when slideBlockType is non-empty; update the code paths that reference body["anchor"] accordingly (look for the block handling around blockID/slideBlockType in drive_add_comment.go).
425-435:resolveCommentTargetfor directslides/sheetis unreachable but keeps a stale message.With
Executefast-pathing bothsheetandslidesvia dedicated functions, andDryRunonly invokingresolveCommentTargetforkind == "wiki", the early-return branch fordocRef.Kind == "sheet" || "slides"here is currently dead. That's fine as defensive code, but the inner checkmode == commentModeLocal && docRef.Kind != "docx" && docRef.Kind != "sheet" && docRef.Kind != "slides"can now only be true whendocRef.Kind == "doc", which makes the broad type list misleading. Consider simplifying to make the contract explicit:♻️ Proposed simplification
- if docRef.Kind == "docx" || docRef.Kind == "doc" || docRef.Kind == "sheet" || docRef.Kind == "slides" { - if mode == commentModeLocal && docRef.Kind != "docx" && docRef.Kind != "sheet" && docRef.Kind != "slides" { - return resolvedCommentTarget{}, output.ErrValidation("local comments only support docx, sheet, and slides; old doc format only supports full comments") - } + if docRef.Kind == "docx" || docRef.Kind == "doc" || docRef.Kind == "sheet" || docRef.Kind == "slides" { + if mode == commentModeLocal && docRef.Kind == "doc" { + return resolvedCommentTarget{}, output.ErrValidation("local comments only support docx, sheet, and slides; old doc format only supports full comments") + } return resolvedCommentTarget{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_add_comment.go` around lines 425 - 435, In resolveCommentTarget simplify the misleading inner condition: replace the compound check (mode == commentModeLocal && docRef.Kind != "docx" && docRef.Kind != "sheet" && docRef.Kind != "slides") with an explicit check for the only remaining case (mode == commentModeLocal && docRef.Kind == "doc"), and update the returned validation error message to clearly state that the old "doc" format only supports full comments; keep the existing resolvedCommentTarget return and references to docRef.Token/FileType/ResolvedBy intact.
🤖 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-drive/SKILL.md`:
- Line 119: Fix the typo and update the table row for local comments: remove the
duplicated Chinese comma after `block_id` and add `sheet` to the list of
supported types, noting that `sheet` also supports local comments via
`--block-id <sheetId>!<cell>` (same as existing `file_token`/`drive
+add-comment` behavior); reference the row mentioning `添加局部评论（划词评论）`,
`file_token`, and the `drive +add-comment`/`--block-id` usage to locate and
apply this change.
---
Outside diff comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 146-187: The dry-run wiki→slides path is broken because
resolveCommentTarget is called with the hard-coded commentModeFull; compute the
actual comment mode used for this invocation (based on the user's
flags/--block-id/--full-comment logic already used elsewhere in this routine)
into a local variable (e.g., mode) and pass that variable to
resolveCommentTarget instead of commentModeFull; update the call in the wiki
branch (where resolveCommentTarget is invoked) to use that computed mode so
slides targets are not rejected and the slides dry-run branch can be taken
(refer to resolveCommentTarget and commentModeFull to locate the change).
In `@skills/lark-drive/references/lark-drive-add-comment.md`:
- Around line 134-148: Update the docs to reflect slides support and flag
validation: change the `--full-comment` description to note it is not valid for
sheet or slides, update the `--block-id` table row to mention both sheet format
(`<sheetId>!<cell>`) and slides format (`<slide-block-type>!<xml-id>`), and
reword the paragraph that currently says "仅支持 docx" (reference the comment about
`--block-id`) so it clarifies that using `--block-id` creates a local/block
comment for docx, sheets, and slides (with slides requiring the
`<slide-block-type>!<xml-id>` form); ensure consistency with the validation
logic implemented in drive_add_comment.go and with the Slide Parameters Mapping
section.
---
Nitpick comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 688-704: The function parseSlidesBlockRef currently allows extra
'!' characters inside the xml-id; update it to validate there is exactly one '!'
separator by checking the count of '!' in blockID and returning the same
validation error if the count is not exactly 1, then split on that single
separator to populate parsedType and parsedID (still trimming and ensuring
neither is empty). Keep the error messages and returned values consistent with
existing output.ErrValidation usage and reference parseSlidesBlockRef, blockID,
parsedType, and parsedID when making the change.
- Around line 670-678: The anchor map mutation in the block handling is
awkwardly done via a chained type assertion on body["anchor"]; instead, after
creating/assigning body["anchor"] = map[string]interface{}{ "block_id": blockID
}, cast body["anchor"] to a local variable (e.g., anchor :=
body["anchor"].(map[string]interface{})) and then set anchor["slide_block_type"]
= strings.TrimSpace(slideBlockType) when slideBlockType is non-empty; update the
code paths that reference body["anchor"] accordingly (look for the block
handling around blockID/slideBlockType in drive_add_comment.go).
- Around line 425-435: In resolveCommentTarget simplify the misleading inner
condition: replace the compound check (mode == commentModeLocal && docRef.Kind
!= "docx" && docRef.Kind != "sheet" && docRef.Kind != "slides") with an explicit
check for the only remaining case (mode == commentModeLocal && docRef.Kind ==
"doc"), and update the returned validation error message to clearly state that
the old "doc" format only supports full comments; keep the existing
resolvedCommentTarget return and references to docRef.Token/FileType/ResolvedBy
intact.
🪄 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: 32fc27e0-096b-44a5-8c5d-dfe2b0f4a0f0
📒 Files selected for processing (4)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-add-comment.md
ethan-zhx
force-pushed
the
feat/slide_comment
branch
from
fa8f5cc to
093fb20
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/drive/drive_add_comment.go (1)
146-153:⚠️ Potential issue | 🔴 CriticalHardcoded
commentModeFullbreaks wiki→slides dry-run.The new guard at lines 454-456 (
objType=="slides" && mode==commentModeFull) errors out, butDryRunalways passescommentModeFulltoresolveCommentTarget. For a wiki URL that resolves toslides(even with a valid--block-id), the resolution silently fails (if err == nilat line 149 short-circuits), soresolvedKindstays"wiki"and the slides branch at line 172 never fires. The dry-run output then incorrectly showsfile_type:"wiki"and"create local comment"instead of the slides path.This will cause
TestDryRunWikiResolvesToSlides(added in this PR) to fail — its assertionstrings.Contains(stdout.String(), "slide block comment")won't match.
Execute(line 279) already does this correctly by computingmodefrom user flags before callingresolveCommentTarget. The same should apply here.🐛 Proposed fix: pass the user-derived mode into wiki resolution
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { docRef, _ := parseCommentDocRef(runtime.Str("doc"), runtime.Str("type")) replyElements, _ := parseCommentReplyElements(runtime.Str("content")) blockID := strings.TrimSpace(runtime.Str("block-id")) + selection := runtime.Str("selection-with-ellipsis") + mode := resolveCommentMode(runtime.Bool("full-comment"), selection, blockID) // For wiki URLs, resolve the actual target type via API so dry-run // matches real execution behavior instead of guessing from --block-id. resolvedKind := docRef.Kind resolvedToken := docRef.Token isWiki := false if docRef.Kind == "wiki" { isWiki = true - target, err := resolveCommentTarget(ctx, runtime, runtime.Str("doc"), commentModeFull) + target, err := resolveCommentTarget(ctx, runtime, runtime.Str("doc"), mode) if err == nil { resolvedKind = target.FileType resolvedToken = target.FileToken } }Then drop the duplicate
selection/modedeclarations later in the function (around line 190-191).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_add_comment.go` around lines 146 - 153, The dry-run uses a hardcoded commentModeFull when calling resolveCommentTarget which prevents wiki URLs that resolve to slides from being recognized; change the DryRun path to compute the comment mode the same way as Execute (derive mode from user flags/selection) and pass that variable into resolveCommentTarget instead of commentModeFull (update the call in the block that sets isWiki/resolvedKind/resolvedToken), then remove the duplicate selection/mode declarations later in the function so there’s a single source of truth for mode used in both DryRun and Execute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 146-153: The dry-run uses a hardcoded commentModeFull when calling
resolveCommentTarget which prevents wiki URLs that resolve to slides from being
recognized; change the DryRun path to compute the comment mode the same way as
Execute (derive mode from user flags/selection) and pass that variable into
resolveCommentTarget instead of commentModeFull (update the call in the block
that sets isWiki/resolvedKind/resolvedToken), then remove the duplicate
selection/mode declarations later in the function so there’s a single source of
truth for mode used in both DryRun and Execute.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5df5cf9f-26c7-4208-9c52-a416186a3821
📒 Files selected for processing (4)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-add-comment.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-drive/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-drive/references/lark-drive-add-comment.md
ethan-zhx
force-pushed
the
feat/slide_comment
branch
from
093fb20 to
1e89d3b
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #674 +/- ##
==========================================
+ Coverage 62.50% 63.02% +0.52%
==========================================
Files 432 436 +4
Lines 37758 38685 +927
==========================================
+ Hits 23599 24381 +782
- Misses 12033 12153 +120
- Partials 2126 2151 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/lark-drive/references/lark-drive-add-comment.md (1)
134-141:⚠️ Potential issue | 🟡 MinorStale docx-only descriptions in the parameter table and behavior bullets — please update for slides.
Three small spots still describe the pre-slides world and contradict the new slides bullets at Lines 143–147:
- Line 134 says
--full-commentis "不适用于 sheet"; slides also disallows it.- Line 135's
--block-idrow only documents the sheet format; the slides format<slide-block-type>!<xml-id>(e.g.shape!bPq) should be listed alongside it for symmetry with the help text.- Line 141 says local comments "仅支持 docx，以及最终可解析为 docx 的 wiki URL"; sheet/slides comments are also
--block-id-driven local comments.Since AI agents typically read the parameter table and behavior bullets first, these inconsistencies are worth tightening up.
📝 Suggested doc tweaks
-| `--block-id` | 局部评论时必填 | 目标块 ID，可通过 `docs +fetch --api-version v2 --detail with-ids` 获取。**Sheet 评论**：格式为 `<sheetId>!<cell>`（如 `a281f9!D6`） | +| `--block-id` | 局部评论时必填 | 目标块 ID。**Docx**：可通过 `docs +fetch --api-version v2 --detail with-ids` 获取。**Sheet**：格式为 `<sheetId>!<cell>`（如 `a281f9!D6`）。**Slides**：格式为 `<slide-block-type>!<xml-id>`（如 `shape!bPq`） |-| `--full-comment` | 否 | 显式指定创建全文评论；未传 `--block-id` 时也会默认走全文评论（不适用于 sheet） | +| `--full-comment` | 否 | 显式指定创建全文评论；未传 `--block-id` 时也会默认走全文评论（不适用于 sheet 和 slides） |-- 未传 `--block-id` 时，shortcut 默认创建**全文评论**；也可以显式传 `--full-comment`。全文评论支持 `docx`、旧版 `doc` URL，以及最终可解析为 `doc`/`docx` 的 wiki URL。 -- 传 `--block-id` 时，shortcut 创建**局部评论（划词评论）**；该模式仅支持 `docx`，以及最终可解析为 `docx` 的 wiki URL。 +- 未传 `--block-id` 时，shortcut 默认创建**全文评论**；也可以显式传 `--full-comment`。全文评论支持 `docx`、旧版 `doc` URL，以及最终可解析为 `doc`/`docx` 的 wiki URL（sheet 和 slides 不支持全文评论）。 +- 传 `--block-id` 时，shortcut 创建**局部评论**：docx 是划词评论（仅支持 `docx` 或解析为 `docx` 的 wiki URL），sheet/slides 走各自的锚点格式（见下方说明）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-drive/references/lark-drive-add-comment.md` around lines 134 - 141, The parameter table and behavior bullets still reference pre-slides rules: update the `--full-comment` row to explicitly state it is not applicable to sheet or slide local comments (so it remains disallowed for sheets/slides), extend the `--block-id` row to document both sheet format `<sheetId>!<cell>` and slide format `<slide-block-type>!<xml-id>` (example: `shape!bPq`) alongside existing guidance, and revise the behavior bullets (the "局部评论需要先获取 block ID" and the paragraph that currently limits local comments to `docx`) to state that `--block-id` drives local (in‑place) comments for docx, sheets, and slides (and for wiki URLs that resolve to those types) instead of saying local comments only support `docx`. Ensure the help text and table entries are consistent and symmetric about sheets vs slides and include the slide example `shape!bPq`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@skills/lark-drive/references/lark-drive-add-comment.md`:
- Around line 134-141: The parameter table and behavior bullets still reference
pre-slides rules: update the `--full-comment` row to explicitly state it is not
applicable to sheet or slide local comments (so it remains disallowed for
sheets/slides), extend the `--block-id` row to document both sheet format
`<sheetId>!<cell>` and slide format `<slide-block-type>!<xml-id>` (example:
`shape!bPq`) alongside existing guidance, and revise the behavior bullets (the
"局部评论需要先获取 block ID" and the paragraph that currently limits local comments to
`docx`) to state that `--block-id` drives local (in‑place) comments for docx,
sheets, and slides (and for wiki URLs that resolve to those types) instead of
saying local comments only support `docx`. Ensure the help text and table
entries are consistent and symmetric about sheets vs slides and include the
slide example `shape!bPq`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8923d65e-e298-4e81-af4a-4bd5d9ca56aa
📒 Files selected for processing (4)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-add-comment.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-drive/SKILL.md
Collaborator
fangshuyu-768
left a comment
fangshuyu-768
left a comment
There was a problem hiding this comment.
One real concern about a flag-validation gap on the wiki→slides path; rest looks solid.
ethan-zhx
force-pushed
the
feat/slide_comment
branch
from
1e89d3b to
212f7ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-drive/references/lark-drive-add-comment.md`:
- Around line 129-135: Update the parameters table to match the Go validator and
prose: change the `--full-comment` row to state it is not applicable to sheet OR
slides (matching Validate), and update the `--block-id` row to document formats
for docx, sheet (`<sheetId>!<cell>`), and slides (`<slide-block-type>!<xml-id>`)
as required for slides comments; ensure the table text aligns with the prose
around line 143 and the Validate logic that rejects `--full-comment` for slides
and requires the slide block-id format.
🪄 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: 7c258fdb-3dd8-4d4f-b570-4ce37cc27ad4
📒 Files selected for processing (4)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-add-comment.md
✅ Files skipped from review due to trivial changes (1)
- shortcuts/drive/drive_add_comment_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-drive/SKILL.md
Change-Id: Id87ecce098d87f7db82389a73f3134b66fcd4814
ethan-zhx
force-pushed
the
feat/slide_comment
branch
from
212f7ce to
e434177
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
shortcuts/drive/drive_add_comment.go (1)
286-290: Wiki resolution metadata is dropped when redirecting toexecuteSlidesComment.When a wiki URL resolves to
slides, onlytarget.FileTokenis forwarded.target.WikiTokenandtarget.ResolvedByare discarded, so the output ofexecuteSlidesComment(lines 826-833) lackswiki_tokenandresolved_by— fields that the docx flow preserves (lines 346, 353). This makes it harder for agents/users to confirm which wiki node was resolved. The same pre-existing gap applies to the sheet redirect at lines 285-287, so this can be addressed consistently for both.♻️ Suggested approach
Pass the full
resolvedCommentTarget(or at leastWikiToken/ResolvedBy) intoexecuteSlidesCommentand include them in the output map when set:if target.FileType == "slides" { return executeSlidesComment(runtime, commentDocRef{Kind: "slides", Token: target.FileToken}, target.WikiToken, target.ResolvedBy) }And in
executeSlidesComment, conditionally add:if wikiToken != "" { out["wiki_token"] = wikiToken } if resolvedBy != "" { out["resolved_by"] = resolvedBy }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_add_comment.go` around lines 286 - 290, The redirect to executeSlidesComment (and executeSheetComment) drops wiki resolution metadata: instead of forwarding only commentDocRef{Kind:"slides", Token: target.FileToken}, pass the full resolvedCommentTarget fields (or at least target.WikiToken and target.ResolvedBy) into executeSlidesComment/executeSheetComment so those functions can include wiki_token and resolved_by in their output map; update the executeSlidesComment signature to accept wikiToken and resolvedBy (or the resolvedCommentTarget) and conditionally add out["wiki_token"] and out["resolved_by"] when non-empty, matching the docx flow behavior.shortcuts/drive/drive_add_comment_test.go (1)
793-827: Consider asserting wiki resolution metadata in stdout.
TestSlidesCommentViaWikiResolveonly checks forcomment_idin stdout. If the slides output is updated to preservewiki_token/resolved_by(see comment on lines 286-290 ofdrive_add_comment.go), extending this test to assert those fields would lock in the contract and prevent silent regressions. As-is, the test passes regardless of whether wiki metadata is preserved or dropped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_add_comment_test.go` around lines 793 - 827, TestSlidesCommentViaWikiResolve currently only asserts the presence of "wikiSlideComment" (comment_id) in stdout; update the test (TestSlidesCommentViaWikiResolve) to also assert that the wiki resolution metadata (e.g., "wiki_token" and "resolved_by") emitted by the DriveAddComment command is present in stdout so regressions that drop wiki metadata are caught. Locate the test's stdout checks and add assertions that stdout contains the expected wiki_token value (presResolved or the token key) and the resolved_by identifier (or whichever field DriveAddComment emits), using the same stub inputs (wikiSlidesToken/presResolved/wikiSlideComment) to form the expected substrings.
🤖 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/drive/drive_add_comment_test.go`:
- Around line 793-827: TestSlidesCommentViaWikiResolve currently only asserts
the presence of "wikiSlideComment" (comment_id) in stdout; update the test
(TestSlidesCommentViaWikiResolve) to also assert that the wiki resolution
metadata (e.g., "wiki_token" and "resolved_by") emitted by the DriveAddComment
command is present in stdout so regressions that drop wiki metadata are caught.
Locate the test's stdout checks and add assertions that stdout contains the
expected wiki_token value (presResolved or the token key) and the resolved_by
identifier (or whichever field DriveAddComment emits), using the same stub
inputs (wikiSlidesToken/presResolved/wikiSlideComment) to form the expected
substrings.
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 286-290: The redirect to executeSlidesComment (and
executeSheetComment) drops wiki resolution metadata: instead of forwarding only
commentDocRef{Kind:"slides", Token: target.FileToken}, pass the full
resolvedCommentTarget fields (or at least target.WikiToken and
target.ResolvedBy) into executeSlidesComment/executeSheetComment so those
functions can include wiki_token and resolved_by in their output map; update the
executeSlidesComment signature to accept wikiToken and resolvedBy (or the
resolvedCommentTarget) and conditionally add out["wiki_token"] and
out["resolved_by"] when non-empty, matching the docx flow behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70dfe367-abbc-46cd-b4db-7a61f4f022e3
📒 Files selected for processing (4)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-add-comment.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-drive/references/lark-drive-add-comment.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-drive/SKILL.md
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
2 participants
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
Extend
drive +add-commentto support slides targets in addition to existing doc/docx flows. This also improves wiki target resolution, validation, dry-run behavior, and skilldocumentation so agents can call the command with clearer and more reliable inputs.
Changes
drive +add-commentdoc/docx/slidestargets before creating comments--block-id <slide-block-type>!<xml-id>--full-commentor--selection-with-ellipsislark-driveskill docs and examples for doc/docx/slides comment creationTest Plan
shortcuts/drive/drive_add_comment_test.golark xxxcommand works as expectedRelated Issues
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes