feat(drive): support sheet cell comments in +add-comment#518
Merged
fangshuyu-768 merged 1 commit intomainfrom Apr 17, 2026
Merged
feat(drive): support sheet cell comments in +add-comment#518fangshuyu-768 merged 1 commit intomainfrom
fangshuyu-768 merged 1 commit intomainfrom
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 spreadsheet ("sheet") comment support to Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / CLI
participant Handler as Command Handler
participant API as Drive API
rect rgba(100,150,200,0.5)
Note over User,API: Direct Sheet URL Path
User->>Handler: drive +add-comment --doc sheets/<token> --block-id <sheetId>!D6
Handler->>Handler: parseSheetCellRef, validate flags
Handler->>API: POST /open-apis/drive/v1/files/<token>/new_comments (anchor: {block_id, sheet_col, sheet_row})
API-->>Handler: 200 OK
Handler-->>User: Success
end
sequenceDiagram
participant User as User / CLI
participant Handler as Command Handler
participant Wiki as Wiki Resolver
participant API as Drive API
rect rgba(200,150,100,0.5)
Note over User,API: Wiki Resolution Path (sheet)
User->>Handler: drive +add-comment --doc wiki/page --block-id <sheetId>!D6
Handler->>Handler: validate flags
Handler->>Wiki: GET /wiki/get_node (resolve)
Wiki-->>Handler: returns file token (FileType=sheet)
Handler->>API: POST /open-apis/drive/v1/files/<token>/new_comments (anchor: {block_id, sheet_col, sheet_row})
API-->>Handler: 200 OK
Handler-->>User: Success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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@03bdef8d777d3abe73fb154155b0090e95dee7db🧩 Skill updatenpx skills add larksuite/cli#feat/sheet_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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/drive/drive_add_comment_test.go`:
- Around line 424-452: Add a new unit test mirroring
TestSheetCommentValidateRejectsFullComment/TestSheetCommentValidateRejectsSelectionWithEllipsis
that asserts the CLI rejects the --block-id flag for sheet comments: create a
TestSheetCommentValidateRejectsBlockID function that calls mountAndRunDrive with
DriveAddComment and args including "+add-comment", "--doc" with a sheet URL,
"--content" JSON, "--sheet-id" "s1", "--sheet-col" "0", "--sheet-row" "0",
"--block-id" "some-id", and "--as" "user", then assert err is non-nil and
err.Error() contains "not applicable for sheet"; follow the same
TestFactory/stdout pattern and error-check style used in the adjacent tests to
ensure consistency.
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 95-110: In Validate (in drive_add_comment.go) parse and
range-check the sheet coordinate flags when docRef.Kind == "sheet": if
runtime.Cmd.Flags().Changed("sheet-col") / "sheet-row" call strconv.Atoi on
runtime.Str("sheet-col") / runtime.Str("sheet-row"), return output.ErrValidation
on non-numeric values, and ensure parsed ints are >= 0 (since flags are
0-based); store the validated ints into the same variables/fields that
Execute/DryRun use (e.g., sheetCol/sheetRow or the request struct) so
DryRun/Execute do not re-parse or fall back to 0. Replace any existing Atoi
calls in Execute/DryRun for sheet-col/sheet-row with usage of the validated
values and ensure errors from strconv.Atoi are surfaced in Validate rather than
masked. Apply the same validation logic to the other sheet-check blocks
referenced (the other Validate/DryRun/Execute sections for sheet comments).
- Around line 256-259: After resolving a wiki target to a concrete type, add a
resolved-type validation before calling executeSheetComment with commentDocRef
to reject incompatible flag combinations: when target.FileType == "sheet" ensure
that doc-only flags (--full-comment, --selection-with-ellipsis, --block-id) are
not set and return the same kind of user-facing error path used elsewhere;
conversely, when resolving to "doc" or "docx" ensure sheet-only flags
(--sheet-id, --sheet-col, --sheet-row) are not set; implement these checks
immediately before the executeSheetComment call (and similarly guard any other
dispatch like executeDocComment if present) so invalid flag sets fail rather
than silently being ignored.
In `@skills/lark-drive/references/lark-drive-add-comment.md`:
- Line 6: Update the doc intro, parameter table, and the behavior note so they
reflect that wiki URLs can resolve to sheets as well as docs/docx: where the
text currently says "支持传最终可解析为 doc/docx 的 wiki URL" change to indicate wiki URLs
may resolve to doc/docx or sheet; adjust descriptions of the flags --sheet-id,
--sheet-col, --sheet-row to state they apply whenever the input resolves to a
sheet (including wiki URLs), and ensure examples/notes that describe
anchor/block handling (anchor.block_id, --selection-with-ellipsis, --block-id)
clarify that full-text comments omit anchor while local comments pass
anchor.block_id to the create_v2
(/open-apis/drive/v1/files/:file_token/new_comments) endpoint; make the same
updates in the other referenced sections (lines ~82-89 and 93) so behavior and
invocation patterns match the implementation and tests.
🪄 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: b001d5c9-34a6-4260-a155-f58ba3a38c82
📒 Files selected for processing (3)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/references/lark-drive-add-comment.md
caojie0621
force-pushed
the
feat/sheet_comment
branch
2 times, most recently
from
2559d22 to
895cace
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
skills/lark-drive/references/lark-drive-add-comment.md (1)
6-6:⚠️ Potential issue | 🟡 MinorThe intro and parameter table still exclude wiki→sheet resolution.
These two spots still say wiki URLs only resolve to
doc/docx, but the implementation and the later behavior note now support wiki nodes that resolve tosheettoo. The page currently contradicts itself and still points users to the wrong invocation pattern.Also applies to: 82-82
🤖 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` at line 6, Update the intro sentence and the parameter table in lark-drive-add-comment.md to reflect that wiki URLs can resolve to sheet nodes as well as doc/docx: change the phrase that currently says "只支持解析为 doc/docx 的 wiki URL" to include "也可解析为 sheet"，and ensure any guidance about invocation patterns (full-comment vs sheet-specific handling) and the later "Behavior" note consistently mention wiki→sheet resolution so the doc no longer contradicts the implementation of create_v2/new_comments handling.shortcuts/drive/drive_add_comment.go (1)
422-432:⚠️ Potential issue | 🟠 MajorDon’t bypass incompatible-flag validation when a wiki resolves to a sheet.
This early
sheetreturn skips the resolved-type guard, so doc-only inputs like--selection-with-ellipsissurvive validation for wiki URLs and then fail later with the unrelated “--block-idis required” error. It also leaves DryRun free to infer the wrong path from the raw flags instead of the resolved target type. Please re-run the same compatibility checks after wiki resolution and add a regression test for the invalid wiki→sheet combinations. As per coding guidelines, “Every behavior change must have an accompanying test”.🤖 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 422 - 432, The early return in the wiki resolution branch bypasses the incompatible-flag validation and DryRun type inference (it returns a resolvedCommentTarget for "sheet" inside the wiki resolution in drive_add_comment.go), so recreate the same compatibility checks used for other resolved types after wiki resolution: instead of returning immediately from the objType == "sheet" block, set the resolvedCommentTarget fields and then call the existing resolved-type guard/validation and DryRun inference (the same logic used in Execute and the resolved-type validation path) to validate flags like --selection-with-ellipsis and --block-id; only return the resolvedCommentTarget if those checks pass, and add a regression test that submits a wiki URL resolving to a sheet with incompatible flags to assert validation fails and DryRun infers the sheet path correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 352-363: Update the validation error message returned by the
branch that checks for unrecognized --doc input so it mentions that wiki URLs
may resolve to doc, docx, or sheet; locate the branch in drive_add_comment.go
that uses extractURLToken and returns output.ErrValidation (the block that
currently checks strings.Contains(raw, "://") and returns unsupported --doc
input) and change the error text to include "wiki→sheet" (e.g., say "use a
doc/docx/sheet URL, a docx token, or a wiki URL that resolves to
doc/docx/sheet") so the guidance reflects wiki→sheet resolution.
- Around line 75-80: The CLI currently omits the explicit sheet-targeting flags
and overloads "block-id" for sheet anchors; add three new flags in the Flags
slice: {Name: "sheet-id", Desc: "sheet identifier", Required: false}, {Name:
"sheet-col", Type: "int", Desc: "0-based sheet column index", Required: false},
{Name: "sheet-row", Type: "int", Desc: "0-based sheet row index", Required:
false}; update the command handler that builds the comment anchor (the code that
reads "block-id" and "selection-with-ellipsis") to prefer the new sheet flags
when all three are provided, validate mutual exclusivity between
sheet-id/col/row and block-id/selection-with-ellipsis, and convert the 0-based
sheet-col/row into the internal sheet anchor format the rest of the code expects
(instead of parsing A1 like "<sheetId>!D6"). Ensure error messages clearly
require all three sheet flags together and include tests or input validation in
the same function to prevent mixed usage.
---
Duplicate comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 422-432: The early return in the wiki resolution branch bypasses
the incompatible-flag validation and DryRun type inference (it returns a
resolvedCommentTarget for "sheet" inside the wiki resolution in
drive_add_comment.go), so recreate the same compatibility checks used for other
resolved types after wiki resolution: instead of returning immediately from the
objType == "sheet" block, set the resolvedCommentTarget fields and then call the
existing resolved-type guard/validation and DryRun inference (the same logic
used in Execute and the resolved-type validation path) to validate flags like
--selection-with-ellipsis and --block-id; only return the resolvedCommentTarget
if those checks pass, and add a regression test that submits a wiki URL
resolving to a sheet with incompatible flags to assert validation fails and
DryRun infers the sheet path correctly.
In `@skills/lark-drive/references/lark-drive-add-comment.md`:
- Line 6: Update the intro sentence and the parameter table in
lark-drive-add-comment.md to reflect that wiki URLs can resolve to sheet nodes
as well as doc/docx: change the phrase that currently says "只支持解析为 doc/docx 的
wiki URL" to include "也可解析为 sheet"，and ensure any guidance about invocation
patterns (full-comment vs sheet-specific handling) and the later "Behavior" note
consistently mention wiki→sheet resolution so the doc no longer contradicts the
implementation of create_v2/new_comments handling.
🪄 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: 5d38fb15-e3f5-4f72-8fce-28e992a5eb17
📒 Files selected for processing (3)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/references/lark-drive-add-comment.md
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/drive/drive_add_comment_test.go
There was a problem hiding this comment.
♻️ Duplicate comments (2)
shortcuts/drive/drive_add_comment.go (2)
361-363:⚠️ Potential issue | 🟡 MinorError message still omits wiki→sheet resolution.
The error message at line 362 states wiki URLs resolve to "doc/docx" but doesn't mention "sheet". Per the past review comment, this should be updated to reflect that wiki URLs can also resolve to sheets.
Suggested fix
if strings.Contains(raw, "://") { - return commentDocRef{}, output.ErrValidation("unsupported --doc input %q: use a doc/docx/sheet URL, a docx token, or a wiki URL that resolves to doc/docx", raw) + return commentDocRef{}, output.ErrValidation("unsupported --doc input %q: use a doc/docx/sheet URL, a docx token, or a wiki URL that resolves to doc/docx/sheet", raw) }🤖 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 361 - 363, Update the validation error text that is returned when the raw --doc input contains "://" so it correctly lists that wiki URLs can resolve to "doc/docx/sheet" (not just "doc/docx"); modify the ErrValidation call in the same block (the return of commentDocRef{} and output.ErrValidation(...)) to include "sheet" in the message and keep the rest of the wording and %q interpolation for the raw variable unchanged.
247-250:⚠️ Potential issue | 🟠 MajorIncompatible flags still not rejected after wiki→sheet resolution.
The past review comment requested validation of incompatible flags after wiki resolution, but I don't see this guard implemented. When a wiki URL resolves to a sheet,
--full-commentand--selection-with-ellipsisare silently ignored instead of returning a validation error.Suggested fix
// Wiki resolved to sheet: redirect to sheet comment path. if target.FileType == "sheet" { + if runtime.Bool("full-comment") || strings.TrimSpace(runtime.Str("selection-with-ellipsis")) != "" { + return output.ErrValidation("--full-comment and --selection-with-ellipsis are not applicable for sheet comments; the wiki resolved to a sheet") + } return executeSheetComment(runtime, commentDocRef{Kind: "sheet", Token: target.FileToken}) }🤖 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 247 - 250, When a wiki URL resolves to a sheet (target.FileType == "sheet") the code jumps to executeSheetComment with commentDocRef{Kind: "sheet", Token: target.FileToken} but does not validate that CLI flags incompatible with sheets (notably --full-comment and --selection-with-ellipsis) are rejected; add a validation step immediately before calling executeSheetComment in the wiki→sheet branch to inspect the runtime/parsed flags (the same flag variables used elsewhere in this file) and return a validation error if --full-comment or --selection-with-ellipsis are set, so these flags are not silently ignored when converting wiki→sheet.
🧹 Nitpick comments (2)
shortcuts/drive/drive_add_comment.go (2)
720-730: Consider includingwiki_tokenin output for wiki→sheet paths.When
executeSheetCommentis called after wiki resolution (line 249), the output doesn't includewiki_token. The regular doc/docx path (line 312-314) includes it. For consistency and traceability, consider passing the wiki token through.Suggested approach
Modify
executeSheetCommentto accept an optional wiki token parameter:-func executeSheetComment(runtime *common.RuntimeContext, docRef commentDocRef) error { +func executeSheetComment(runtime *common.RuntimeContext, docRef commentDocRef, wikiToken string) error { // ... existing code ... out := map[string]interface{}{ // ... existing fields ... } + if wikiToken != "" { + out["wiki_token"] = wikiToken + } runtime.Out(out, nil) return nil }Then update call sites:
- Line 235:
executeSheetComment(runtime, docRef, "")- Line 249:
executeSheetComment(runtime, commentDocRef{Kind: "sheet", Token: target.FileToken}, target.WikiToken)🤖 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 720 - 730, The sheet comment output omits wiki_token for wiki→sheet flows; update executeSheetComment to accept an optional wikiToken parameter (e.g., func executeSheetComment(runtime, docRef commentDocRef, wikiToken string) ), add wiki_token to the out map when wikiToken is non-empty, and update callers to pass "" for non-wiki paths and pass target.WikiToken for the wiki resolution path (e.g., executeSheetComment(runtime, commentDocRef{Kind:"sheet", Token:target.FileToken}, target.WikiToken)). Ensure the function signature and all call sites compile after the change.
129-133: Wiki +!heuristic may misfire in edge cases.The condition
docRef.Kind == "wiki" && strings.Contains(blockID, "!")assumes that!in--block-idalways indicates a sheet cell reference. While unlikely, a docx block ID containing!would incorrectly trigger the sheet path in DryRun. Since this only affects the preview (not execution), the risk is low.🤖 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 129 - 133, The heuristic should only take the sheet path when the blockID actually parses as a sheet cell reference; update the condition in the DryRun branch so you call parseSheetCellRef(blockID) and proceed to the sheet branch only if docRef.Kind == "sheet" OR (docRef.Kind == "wiki" AND parseSheetCellRef returned a non-nil anchor). Reference the existing symbols: parseSheetCellRef, sheetAnchor, docRef.Kind and blockID; ensure you remove the sole strings.Contains(blockID, "!") check and default to the existing placeholder anchor only when parseSheetCellRef returns nil after confirming the branch should be treated as a sheet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 361-363: Update the validation error text that is returned when
the raw --doc input contains "://" so it correctly lists that wiki URLs can
resolve to "doc/docx/sheet" (not just "doc/docx"); modify the ErrValidation call
in the same block (the return of commentDocRef{} and output.ErrValidation(...))
to include "sheet" in the message and keep the rest of the wording and %q
interpolation for the raw variable unchanged.
- Around line 247-250: When a wiki URL resolves to a sheet (target.FileType ==
"sheet") the code jumps to executeSheetComment with commentDocRef{Kind: "sheet",
Token: target.FileToken} but does not validate that CLI flags incompatible with
sheets (notably --full-comment and --selection-with-ellipsis) are rejected; add
a validation step immediately before calling executeSheetComment in the
wiki→sheet branch to inspect the runtime/parsed flags (the same flag variables
used elsewhere in this file) and return a validation error if --full-comment or
--selection-with-ellipsis are set, so these flags are not silently ignored when
converting wiki→sheet.
---
Nitpick comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 720-730: The sheet comment output omits wiki_token for wiki→sheet
flows; update executeSheetComment to accept an optional wikiToken parameter
(e.g., func executeSheetComment(runtime, docRef commentDocRef, wikiToken string)
), add wiki_token to the out map when wikiToken is non-empty, and update callers
to pass "" for non-wiki paths and pass target.WikiToken for the wiki resolution
path (e.g., executeSheetComment(runtime, commentDocRef{Kind:"sheet",
Token:target.FileToken}, target.WikiToken)). Ensure the function signature and
all call sites compile after the change.
- Around line 129-133: The heuristic should only take the sheet path when the
blockID actually parses as a sheet cell reference; update the condition in the
DryRun branch so you call parseSheetCellRef(blockID) and proceed to the sheet
branch only if docRef.Kind == "sheet" OR (docRef.Kind == "wiki" AND
parseSheetCellRef returned a non-nil anchor). Reference the existing symbols:
parseSheetCellRef, sheetAnchor, docRef.Kind and blockID; ensure you remove the
sole strings.Contains(blockID, "!") check and default to the existing
placeholder anchor only when parseSheetCellRef returns nil after confirming the
branch should be treated as a sheet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b03b0fec-fc97-4f9e-b2f8-9cec8b5f6592
📒 Files selected for processing (3)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/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)
- shortcuts/drive/drive_add_comment_test.go
caojie0621
force-pushed
the
feat/sheet_comment
branch
from
895cace to
27c863e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
shortcuts/drive/drive_add_comment.go (2)
247-250:⚠️ Potential issue | 🟠 MajorRevalidate doc-only flags after wiki resolves to
sheet.This redirect bypasses the direct-sheet validation path. A wiki URL that resolves to
sheetstill accepts--full-commentor--selection-with-ellipsis, and thenexecuteSheetCommentturns that into a misleading--block-id is requirederror or silently ignores the selection intent. Please reject those combinations before dispatching here.Suggested guard
// Wiki resolved to sheet: redirect to sheet comment path. if target.FileType == "sheet" { + if runtime.Bool("full-comment") || strings.TrimSpace(runtime.Str("selection-with-ellipsis")) != "" { + return output.ErrValidation("--full-comment and --selection-with-ellipsis are not applicable for sheet comments; use --block-id with <sheetId>!<cell> format") + } return executeSheetComment(runtime, commentDocRef{Kind: "sheet", Token: target.FileToken}) }🤖 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 247 - 250, The code redirects wiki targets resolved to sheet by calling executeSheetComment(commentDocRef{Kind: "sheet", Token: target.FileToken}) without revalidating doc-only flags; update the logic around the target.FileType == "sheet" branch to re-check and reject incompatible flags (e.g., --full-comment and --selection-with-ellipsis) before dispatching to executeSheetComment, returning a clear validation error (mentioning that --block-id is required for sheets or disallowing the selection/full-comment options) so executeSheetComment no longer receives invalid combinations.
127-150:⚠️ Potential issue | 🟡 MinorDon't infer wiki→sheet DryRun from
--block-idsyntax alone.For wiki targets this can preview the wrong flow:
--block-id s1!A1forces a sheet preview even whenget_nodewould resolve todocx, while a wiki that actually resolves tosheetstill falls back to the doc/docx preview when--block-idis missing or malformed. That makes DryRun diverge from Execute for the same argv.Based on learnings,
Validateis always executed beforeDryRun, andDryRuncannot surface validation failures.🤖 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 127 - 150, The DryRun currently infers a wiki→sheet two-step flow from --block-id syntax (strings.Contains(blockID,"!")) which can diverge from actual execution; instead, change the logic so docRef.Kind == "sheet" alone triggers the single-step sheet POST, and any docRef.Kind == "wiki" always produces the two-step DryRun that first GETs /open-apis/wiki/v2/spaces/get_node and then POSTs to drive with file_token "<resolved_sheet_token>" (i.e., always set desc = "2-step orchestration: resolve wiki -> create sheet comment" and fileToken = "<resolved_sheet_token>" for wiki); keep using parseSheetCellRef and buildCommentCreateV2Request to optionally populate anchor but do not let blockID syntax determine whether to render the wiki resolution step (update the block that builds commentBody, dry := common.NewDryRunAPI().Desc(desc), the conditional that adds dry.GET(...), and the final return dry.POST(...).)
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/drive/drive_add_comment_test.go`:
- Around line 307-608: The test suite is missing DryRun coverage for the new
sheet and wiki→sheet paths; add E2E dry-run tests that exercise
DriveAddComment.DryRun so the preview branch is exercised. Implement new tests
in this file (or corresponding dryrun e2e folder) that call mountAndRunDrive
with DriveAddComment and the same args used in TestSheetCommentExecuteSuccess,
TestSheetCommentViaWikiResolve, etc., but include the "--dry-run" flag and
assert the command returns success and that stdout contains the expected preview
output (e.g., contains the generated comment_id or preview markers) and that no
POST to /open-apis/drive/v1/files/.../new_comments is performed (register no
POST stub or assert no network call). Ensure tests cover both direct sheet URLs
("--doc" with sheets token and "--block-id" like "s1!D6") and wiki URL
resolution path ("--doc" with wiki token and resolved sheet block-id) to
validate both branches in DriveAddComment.DryRun.
---
Duplicate comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 247-250: The code redirects wiki targets resolved to sheet by
calling executeSheetComment(commentDocRef{Kind: "sheet", Token:
target.FileToken}) without revalidating doc-only flags; update the logic around
the target.FileType == "sheet" branch to re-check and reject incompatible flags
(e.g., --full-comment and --selection-with-ellipsis) before dispatching to
executeSheetComment, returning a clear validation error (mentioning that
--block-id is required for sheets or disallowing the selection/full-comment
options) so executeSheetComment no longer receives invalid combinations.
- Around line 127-150: The DryRun currently infers a wiki→sheet two-step flow
from --block-id syntax (strings.Contains(blockID,"!")) which can diverge from
actual execution; instead, change the logic so docRef.Kind == "sheet" alone
triggers the single-step sheet POST, and any docRef.Kind == "wiki" always
produces the two-step DryRun that first GETs /open-apis/wiki/v2/spaces/get_node
and then POSTs to drive with file_token "<resolved_sheet_token>" (i.e., always
set desc = "2-step orchestration: resolve wiki -> create sheet comment" and
fileToken = "<resolved_sheet_token>" for wiki); keep using parseSheetCellRef and
buildCommentCreateV2Request to optionally populate anchor but do not let blockID
syntax determine whether to render the wiki resolution step (update the block
that builds commentBody, dry := common.NewDryRunAPI().Desc(desc), the
conditional that adds dry.GET(...), and the final return dry.POST(...).)
🪄 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: 228bf966-8576-4ad8-a27d-d8bb7a6dfe13
📒 Files selected for processing (3)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/references/lark-drive-add-comment.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-drive/references/lark-drive-add-comment.md
caojie0621
force-pushed
the
feat/sheet_comment
branch
from
27c863e to
d1f4d9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
shortcuts/drive/drive_add_comment_test.go (1)
552-586: Extend wiki→sheet success test to assert wiki context in the output.Once the wiki→sheet execute path starts emitting
resolved_by/wiki_token(see the comment ondrive_add_comment.goline 248-251), this test should additionally assert thatstdoutcontains"resolved_by":"wiki"and the wiki token, so the wiki provenance is locked in and future regressions are caught.🤖 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 552 - 586, Update the TestSheetCommentViaWikiResolve test to assert the wiki provenance fields emitted by the wiki→sheet path: after the existing comment_id check in TestSheetCommentViaWikiResolve (which invokes mountAndRunDrive with DriveAddComment), add assertions that stdout contains `"resolved_by":"wiki"` and the original wiki token string (e.g. `wikiToken123`) to ensure the output includes the wiki context emitted by the code in drive_add_comment.go.shortcuts/drive/drive_add_comment.go (1)
663-698:parseSheetCellRefLGTM; minor: unvalidated column overflow.Logic is correct for A1-style cells (0-based col/row, lowercase handled, row<1 rejected, empty/missing parts rejected). One small gap:
col = col*26 + int(ch-'A'+1)has no upper bound, so a pathological input likeA...!1(many letters) would overflow int. Realistic sheets cap atXFD, so not urgent — consider cappinglen(colStr)at 3 (or validating against a reasonable max column) for defense in depth.🤖 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 663 - 698, The parseSheetCellRef function currently converts colStr to an integer without bounds checking, which can overflow for excessively long column letters; update parseSheetCellRef to validate colStr length and range before/after conversion (e.g. reject len(colStr) > 3 or compute col and ensure col <= 16383 for XFD), returning an output.ErrValidation with a clear message when the column is out of range; reference colStr, col, and parseSheetCellRef when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 248-251: The fast-path for Wiki→sheet resolution drops wiki
context by creating a new commentDocRef with only Kind and Token and calling
executeSheetComment, which causes output to lose target.ResolvedBy and
target.WikiToken; update the call site and/or commentDocRef to preserve and
forward the resolved metadata (e.g., add ResolvedBy, WikiToken and DocID fields
to commentDocRef or add extra params to executeSheetComment) and update
executeSheetComment to include those fields in its emitted envelope so
resolved_by/wiki_token/doc_id survive when target.FileType == "sheet".
---
Nitpick comments:
In `@shortcuts/drive/drive_add_comment_test.go`:
- Around line 552-586: Update the TestSheetCommentViaWikiResolve test to assert
the wiki provenance fields emitted by the wiki→sheet path: after the existing
comment_id check in TestSheetCommentViaWikiResolve (which invokes
mountAndRunDrive with DriveAddComment), add assertions that stdout contains
`"resolved_by":"wiki"` and the original wiki token string (e.g. `wikiToken123`)
to ensure the output includes the wiki context emitted by the code in
drive_add_comment.go.
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 663-698: The parseSheetCellRef function currently converts colStr
to an integer without bounds checking, which can overflow for excessively long
column letters; update parseSheetCellRef to validate colStr length and range
before/after conversion (e.g. reject len(colStr) > 3 or compute col and ensure
col <= 16383 for XFD), returning an output.ErrValidation with a clear message
when the column is out of range; reference colStr, col, and parseSheetCellRef
when making the change.
🪄 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: 66b62efd-93e5-4ae2-9639-36ce623cb900
⛔ Files ignored due to path filters (1)
span.logis excluded by!**/*.log
📒 Files selected for processing (3)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/references/lark-drive-add-comment.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-drive/references/lark-drive-add-comment.md
caojie0621
force-pushed
the
feat/sheet_comment
branch
2 times, most recently
from
d85fe1c to
403cf30
Compare
Extend +add-comment to accept sheet URLs and wiki URLs that resolve to sheets. Reuse --block-id with <sheetId>!<cell> format (e.g. a281f9!D6) for sheet cell positioning. Wiki links resolving to sheet type are handled by first calling get_node, then redirecting to the sheet comment path with proper parameter validation.
caojie0621
force-pushed
the
feat/sheet_comment
branch
from
403cf30 to
03bdef8
Compare
fangshuyu-768
approved these changes
Apr 17, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #518 +/- ##
=======================================
Coverage ? 59.52%
=======================================
Files ? 384
Lines ? 32722
Branches ? 0
=======================================
Hits ? 19478
Misses ? 11412
Partials ? 1832 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
+add-commentto support adding comments to spreadsheet cells.Accepts sheet URLs and wiki URLs that resolve to sheets, reusing
--block-idwith<sheetId>!<cell>format (e.g.a281f9!D6) forcell positioning.
Changes
parseCommentDocRefsupports/sheets/URL parsingresolveCommentTargetallows wiki nodes resolving tosheettypeparseSheetCellRefparses<sheetId>!<cell>into sheet comment anchorexecuteSheetCommenthandles sheet comment creationbuildCommentCreateV2Requestaccepts optional sheet anchor parameter--full-comment/--selection-with-ellipsisfor sheetslark-drive-add-comment.mdwith sheet examples and parametersTest Plan
go buildandgo vetpassparseSheetCellRef(5 valid + 6 invalid cases)--block-id, invalid format, rejects--full-comment,rejects
--selection-with-ellipsis--block-idRelated Issues
N/A
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests