feat: add drive list comments shortcut#1845
Conversation
|
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 the ChangesDrive list-comments shortcut
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant DriveListComments
participant WikiAPI
participant DriveCommentsAPI
CLI->>DriveListComments: Execute with URL or token
DriveListComments->>WikiAPI: Resolve wiki target when needed
WikiAPI-->>DriveListComments: Return underlying token and type
DriveListComments->>DriveCommentsAPI: List comments with filters
DriveCommentsAPI-->>DriveListComments: Return comments and pagination
DriveListComments-->>CLI: Emit normalized output
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1845 +/- ##
==========================================
- Coverage 74.58% 74.57% -0.02%
==========================================
Files 863 864 +1
Lines 90123 90315 +192
==========================================
+ Hits 67220 67352 +132
- Misses 17696 17743 +47
- Partials 5207 5220 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9274025640ad5ee61a893766ec008dee16cee4b7🧩 Skill updatenpx skills add larksuite/cli#feat/drive-list-comments-shortcut -y -g |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
shortcuts/drive/drive_list_comments_test.go (1)
128-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding dedicated tests for
validateDriveListCommentsSpecerror paths.The validation function covers four error paths (invalid page-size, invalid solved-status, invalid comment-scope, invalid user-id-type), but none are directly tested. While the happy paths are indirectly covered through the execution tests, the error paths have no coverage. Adding a small table-driven test would ensure these validations don't regress.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/drive/drive_list_comments_test.go` around lines 128 - 142, The current tests cover the happy path for buildDriveListCommentsParams, but validateDriveListCommentsSpec error branches are untested. Add a small table-driven test that calls validateDriveListCommentsSpec directly and asserts errors for the four invalid cases: page-size, solved-status, comment-scope, and user-id-type. Keep it close to the existing drive list comments tests so the new coverage is easy to find alongside buildDriveListCommentsParams.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/drive/drive_list_comments_test.go`:
- Around line 75-79: The error assertions in TestResolveDriveListCommentsInput
only check the message substring, but the validation paths in
resolveDriveListCommentsInput return typed errs.NewValidationError values.
Update the test to use errors.As against *errs.ValidationError and assert the
expected Category, SubtypeInvalidArgument, Param, and that the cause is
preserved, while keeping the existing message check if needed. Add the required
errors and github.com/larksuite/cli/errs imports so the test validates typed
metadata instead of relying on strings.Contains alone.
In `@skills/lark-drive/references/lark-drive-comments-guide.md`:
- Around line 68-69: Update the `drive +list-comments` guidance in the comments
guide to make it clear that “all comments” only applies when `--solved-status
all` is passed; otherwise the shortcut defaults to unresolved comments. Adjust
the wording in the `+list-comments` description so it does not imply
full-history counting, sorting, or “latest/last N” retrieval by default, while
keeping the distinction from the raw `drive file.comments list` command.
---
Nitpick comments:
In `@shortcuts/drive/drive_list_comments_test.go`:
- Around line 128-142: The current tests cover the happy path for
buildDriveListCommentsParams, but validateDriveListCommentsSpec error branches
are untested. Add a small table-driven test that calls
validateDriveListCommentsSpec directly and asserts errors for the four invalid
cases: page-size, solved-status, comment-scope, and user-id-type. Keep it close
to the existing drive list comments tests so the new coverage is easy to find
alongside buildDriveListCommentsParams.
🪄 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: 38db4354-78e2-42d6-96ee-fc8be4e9bc11
📒 Files selected for processing (11)
shortcuts/drive/drive_list_comments.goshortcuts/drive/drive_list_comments_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-comment-location.mdskills/lark-drive/references/lark-drive-comments-guide.mdskills/lark-drive/references/lark-drive-list-comments.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_add_comment_workflow_test.gotests/cli_e2e/drive/drive_list_comments_dryrun_test.go
5fbabc7 to
9d25990
Compare
bbaaeff to
35d130c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-drive/SKILL.md`:
- Around line 84-91: The 评论能力入口 section only documents URL and file_token
inputs; update it to document the exact raw token/type invocation supported by
+list-comments. Clarify that raw Wiki tokens require the Wiki type and differ
from the underlying file token, while preserving the existing URL
auto-resolution guidance.
🪄 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: b6cf4de2-9464-456f-a48d-9fbb86e570da
📒 Files selected for processing (13)
shortcuts/drive/drive_list_comments.goshortcuts/drive/drive_list_comments_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-comment-location.mdskills/lark-drive/references/lark-drive-comments-guide.mdskills/lark-drive/references/lark-drive-list-comments.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_add_comment_workflow_test.gotests/cli_e2e/drive/drive_list_comments_dryrun_test.gotests/cli_e2e/drive/helpers.gotests/cli_e2e/drive/helpers_test.go
✅ Files skipped from review due to trivial changes (3)
- shortcuts/drive/shortcuts.go
- tests/cli_e2e/drive/coverage.md
- skills/lark-drive/references/lark-drive-comment-location.md
🚧 Files skipped from review as they are similar to previous changes (7)
- skills/lark-drive/references/lark-drive-list-comments.md
- tests/cli_e2e/drive/drive_add_comment_workflow_test.go
- shortcuts/drive/shortcuts_test.go
- skills/lark-drive/references/lark-drive-comments-guide.md
- tests/cli_e2e/drive/drive_list_comments_dryrun_test.go
- shortcuts/drive/drive_list_comments_test.go
- shortcuts/drive/drive_list_comments.go
9a12e67 to
fbf3954
Compare
fbf3954 to
9274025
Compare
* fix: unify dry-run output contract
* fix: address dry-run review feedback
* fix(dryrun): tighten preview contract and unify data shape
- transcribe HTTP method verbatim in previews (HEAD/OPTIONS were
reported as GET); reject an empty method in api with a typed error
- unify the dry-run data payload across api/service/shortcut paths:
{api, context?: {app_id, user_open_id}}; drop data.as — the envelope
top-level identity is the single identity source
- mark pretty dry-run stdout with '# dry-run: request not sent' so logs
that drop stderr still show it was a preview
- extract the shared preview builder, collapse PrintDryRunWithFile's
loose params into FileUploadMeta, and fail loudly on nil previews
- revert description-marker identity parsing: stale prose must not
override corrected accessTokens (blocks legal user calls on
images.create); identity gating keys off accessTokens only
- pin the new contracts with tests: verbatim method, three-way context
parity, nil-preview error, empty-context omission, marker line
* docs(agents): add typed-data, faithful-transcription, and contract-test conventions
- typed struct at the boundary over map[string]interface{} threading;
distinct types where values could swap silently (internal/meta.Token)
- transcribe input verbatim in previews/transformations; reject
unhonorable flag combinations with typed errors instead of silently
substituting behavior
- contract tests must fail when the implementation is reverted
* test: migrate dry-run tests grown on main to the envelope format
main gained raw-format dry-run readers while the PR was in flight
(wiki drive export #1802, drive list comments #1845, slash commands,
sheets history, docs fetch, mail draft-send/triage, vc meeting events).
Migrate them to the envelope accessors (clie2e.DryRunGet / data-wrapped
decoders) and drop the now-redundant DryRunData extractions in files
unified on DryRunGet.
---------
Co-authored-by: guokexin.02 <264159873+Tantanz20020918@users.noreply.github.com>
Summary
Add
drive +list-commentsas the preferred shortcut for listing document comments. The shortcut accepts URLs or token/type inputs, unwraps Wiki tokens to the underlying document token/type, and exposes explicit filters for solved status, comment scope, pagination, reactions, and docx relation data.Changes
drive +list-commentswith URL parsing, Wiki node resolution, typed validation, dry-run orchestration, and structured output.lark-driveskills to preferdrive +list-comments, document URL-first usage, Wiki support,--solved-status,--comment-scope, and non-docx--need-relationbehavior.Test Plan
go test ./shortcuts/drive -run 'Test(ResolveDriveListCommentsInput|BuildDriveListCommentsParams|DriveListCommentsExecuteDocx|DriveListCommentsExecuteWikiResolvesToDocx|ShortcutsIncludesExpectedCommands)$'go test -count=1 ./tests/cli_e2e/drive -run 'TestDriveListCommentsDryRun'lark-cli drive +list-commentsflow works as expected.Test report and retained test data: https://bytedance.larkoffice.com/wiki/ChV2wyAwdiIXfPkyMKncn4idnnf
Related Issues
Summary by CodeRabbit
drive +list-commentsto list document comments using--urlor--token(with wiki auto-resolution), including pagination and filters for solved status and comment scope.--need-relationto return DOCX-specific comment location/relationship details (non-DOCX types ignore it).--dry-runrequest plans.--need-relationlimitations.