Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded an optional Changes
Sequence Diagram(s)(Skipped — changes are minor flag+request-body handling, tests, and docs; no new multi-component flow requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds an optional Confidence Score: 5/5Safe to merge; all remaining findings are minor suggestions that do not affect correctness. The core implementation is correct — shortcuts/task/tasklist_add_task_test.go — test coverage for the section_guid request body is still incomplete. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as lark-cli (Execute)
participant API as Lark Task API
User->>CLI: +tasklist-task-add --tasklist-id tl-123 --task-id task-1,task-2 --section-guid sec-456
CLI->>CLI: extractTasklistGuid("tl-123")
CLI->>CLI: build body {tasklist_guid, section_guid}
loop for each task-id
CLI->>API: POST /open-apis/task/v2/tasks/{taskId}/add_tasklist
note right of API: body: {tasklist_guid, section_guid}
API-->>CLI: {code:0, data:{task:{guid,url}}}
CLI->>CLI: append to successful / failed
end
CLI-->>User: JSON {successful_tasks, failed_tasks, tasklist_guid}
Reviews (3): Last reviewed commit: "Merge branch 'feat/tasklist-add-section-..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
shortcuts/task/tasklist_add_task.go (1)
44-46: Trim--section-guidbefore deciding whether to send it.A whitespace-only flag value currently passes the non-empty check and can produce an invalid
section_guidpayload.Proposed fix
- if sectionGuid := runtime.Str("section-guid"); sectionGuid != "" { + if sectionGuid := strings.TrimSpace(runtime.Str("section-guid")); sectionGuid != "" { body["section_guid"] = sectionGuid } ... - if sectionGuid := runtime.Str("section-guid"); sectionGuid != "" { + if sectionGuid := strings.TrimSpace(runtime.Str("section-guid")); sectionGuid != "" { body["section_guid"] = sectionGuid }Also applies to: 65-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/task/tasklist_add_task.go` around lines 44 - 46, Trim the --section-guid flag value before deciding to include it in the payload: call strings.TrimSpace on runtime.Str("section-guid") (e.g., assign trimmed := strings.TrimSpace(runtime.Str("section-guid"))) and only set body["section_guid"] = trimmed when trimmed != ""; apply the same change to the second occurrence at the other runtime.Str("section-guid") check (lines around the body assignment at 65-67) and add the strings import if missing.shortcuts/task/tasklist_add_task_test.go (1)
14-25: Test does not verify the newsection-guidbehavior end-to-end.The test passes
--section-guidbut only assertstasklist_guidin output. Please also assert the outgoing request body containssection_guid, otherwise regressions in the new feature can slip through.Also applies to: 30-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/task/tasklist_add_task_test.go` around lines 14 - 25, The test registers an httpmock.Stub (reg.Register(&httpmock.Stub{...})) for the POST add_tasklist call but never verifies the outgoing request includes the new "section_guid" field when the test runs with --section-guid; update the stub or the test harness to assert the request body contains "section_guid" with the expected value (e.g. add a Body matcher or inspect the recorded request in the test and assert reqBody["section_guid"] == "expected-guid") in the tasklist_add_task_test.go test (and repeat the same assertion for the other similar cases referenced around lines 30-38) so the feature is exercised end-to-end.
🤖 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/task/tasklist_add_task_test.go`:
- Line 1: Add the required project license header at the top of the test file
before the package declaration to satisfy license-eye; update
shortcuts/task/tasklist_add_task_test.go by inserting the standard license block
above the existing "package task" line so the file begins with the approved
license header followed by the package declaration.
---
Nitpick comments:
In `@shortcuts/task/tasklist_add_task_test.go`:
- Around line 14-25: The test registers an httpmock.Stub
(reg.Register(&httpmock.Stub{...})) for the POST add_tasklist call but never
verifies the outgoing request includes the new "section_guid" field when the
test runs with --section-guid; update the stub or the test harness to assert the
request body contains "section_guid" with the expected value (e.g. add a Body
matcher or inspect the recorded request in the test and assert
reqBody["section_guid"] == "expected-guid") in the tasklist_add_task_test.go
test (and repeat the same assertion for the other similar cases referenced
around lines 30-38) so the feature is exercised end-to-end.
In `@shortcuts/task/tasklist_add_task.go`:
- Around line 44-46: Trim the --section-guid flag value before deciding to
include it in the payload: call strings.TrimSpace on runtime.Str("section-guid")
(e.g., assign trimmed := strings.TrimSpace(runtime.Str("section-guid"))) and
only set body["section_guid"] = trimmed when trimmed != ""; apply the same
change to the second occurrence at the other runtime.Str("section-guid") check
(lines around the body assignment at 65-67) and add the strings import if
missing.
🪄 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: a98aaf10-ed16-4124-aacb-27d77748b4a0
📒 Files selected for processing (4)
shortcuts/task/tasklist_add_task.goshortcuts/task/tasklist_add_task_test.goskills/lark-task/SKILL.mdskills/lark-task/references/lark-task-tasklist-task-add.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@065fc0e34f3f3c24e0e01090e8a141d438b24ce6🧩 Skill updatenpx skills add zero-my/cli#feat/task-section-updates -y -g |
aec407b to
a55c04d
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 `@shortcuts/task/tasklist_add_task.go`:
- Around line 44-47: Trim the value returned by runtime.Str("section-guid")
before checking emptiness and adding body["section_guid"]; i.e., assign a
trimmed variable (e.g., sectionGuid :=
strings.TrimSpace(runtime.Str("section-guid"))) then if sectionGuid != "" set
body["section_guid"] = sectionGuid. Apply the same pattern to the other
conditional that reads a runtime.Str and sets a body field (the second
occurrence around the other body[...] assignment) so whitespace-only input is
rejected there as well.
🪄 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: 44807091-7e59-4e19-bfa2-57ddc2fdcbae
📒 Files selected for processing (3)
shortcuts/task/tasklist_add_task.goshortcuts/task/tasklist_add_task_test.goskills/lark-task/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/lark-task/SKILL.md
- shortcuts/task/tasklist_add_task_test.go
a55c04d to
065fc0e
Compare
Summary
Changes
Test Plan
lark xxxcommand works as expectedRelated Issues
Summary by CodeRabbit
New Features
Documentation
Tests