docs: tighten lark-sheets skill guidance#1286
Conversation
📝 WalkthroughWalkthroughUpdates to skills/lark-sheets documentation: clarify scope and routing, add Changeslark-sheets Skill Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1286 +/- ##
==========================================
+ Coverage 70.18% 70.33% +0.14%
==========================================
Files 671 672 +1
Lines 65240 65322 +82
==========================================
+ Hits 45786 45941 +155
+ Misses 15783 15728 -55
+ Partials 3671 3653 -18 ☔ 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@1b1193be95887f11d80c486b3b380cc1aeda94b9🧩 Skill updatenpx skills add larksuite/cli#codex/optimize-lark-sheets-skill -y -g |
51716ef to
5dbb4e3
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-sheets/SKILL.md`:
- Line 16: Update the SKILL.md guidance to require the auth login command
include a scope or domain flag: change the sentence that currently instructs
users to run `lark-cli auth login` to explicitly show that `lark-cli auth login
--domain <your-domain>` or `lark-cli auth login --scope <your-scope>` (or
equivalent) must be supplied; reference the auth flow rule from
lark-shared/SKILL.md and ensure the phrase containing `lark-cli auth login` is
replaced or appended with the required `--domain`/`--scope` note so users won’t
run the bare command and hit an error.
🪄 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: 4ce13a87-ef0b-4d3f-a0fe-2bd0a3a9fe6e
📒 Files selected for processing (1)
skills/lark-sheets/SKILL.md
|
|
||
| **CRITICAL — 开始前 MUST 先用 Read 工具读取 [`../lark-shared/SKILL.md`](../lark-shared/SKILL.md),其中包含认证、权限处理。** | ||
|
|
||
| **身份:电子表格通常属于用户云空间资源,优先使用 `--as user`。首次使用前执行 `lark-cli auth login`。** |
There was a problem hiding this comment.
Incomplete auth login guidance.
The instruction states "首次使用前执行 lark-cli auth login" without specifying that --domain or --scope is required. Per lark-shared/SKILL.md, auth login must specify scope or domain: "auth login 必须指定范围(--domain 或 --scope)". Users running the bare command will encounter an error.
Consider adding the required parameter:
📝 Suggested clarification
-**身份:电子表格通常属于用户云空间资源,优先使用 `--as user`。首次使用前执行 `lark-cli auth login`。**
+**身份:电子表格通常属于用户云空间资源,优先使用 `--as user`。首次使用前需执行 `lark-cli auth login --scope "<scope>"` 或 `--domain <domain>`(详见 [`lark-shared/SKILL.md`](../lark-shared/SKILL.md))。**Based on learnings from lark-shared/SKILL.md auth flow, which documents that auth login requires --domain or --scope specification.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **身份:电子表格通常属于用户云空间资源,优先使用 `--as user`。首次使用前执行 `lark-cli auth login`。** | |
| **身份:电子表格通常属于用户云空间资源,优先使用 `--as user`。首次使用前需执行 `lark-cli auth login --scope "<scope>"` 或 `--domain <domain>`(详见 [`lark-shared/SKILL.md`](../lark-shared/SKILL.md))。** |
🤖 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 `@skills/lark-sheets/SKILL.md` at line 16, Update the SKILL.md guidance to
require the auth login command include a scope or domain flag: change the
sentence that currently instructs users to run `lark-cli auth login` to
explicitly show that `lark-cli auth login --domain <your-domain>` or `lark-cli
auth login --scope <your-scope>` (or equivalent) must be supplied; reference the
auth flow rule from lark-shared/SKILL.md and ensure the phrase containing
`lark-cli auth login` is replaced or appended with the required
`--domain`/`--scope` note so users won’t run the bare command and hit an error.
5dbb4e3 to
ab523f0
Compare
ab523f0 to
1b1193b
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
skills/lark-sheets/SKILL.md (1)
16-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIncomplete auth login guidance.
The instruction states "首次使用前执行
lark-cli auth login" without specifying that--domainor--scopeis required. Perlark-shared/SKILL.md, auth login must specify scope or domain. Users running the bare command will encounter an error.Consider adding the required parameter guidance.
Based on learnings from lark-shared/SKILL.md auth flow, which documents that auth login requires --domain or --scope specification.
🤖 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 `@skills/lark-sheets/SKILL.md` at line 16, The auth guidance in SKILL.md currently tells users to run "lark-cli auth login" but omits required parameters; update the sentence to instruct users to include either the --domain or --scope flag (for example: lark-cli auth login --domain <your-domain> or --scope <scope>) so the auth flow accepts the request, referencing the same auth flow rules from lark-shared/SKILL.md and the "lark-cli auth login" command in the file.
🤖 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.
Duplicate comments:
In `@skills/lark-sheets/SKILL.md`:
- Line 16: The auth guidance in SKILL.md currently tells users to run "lark-cli
auth login" but omits required parameters; update the sentence to instruct users
to include either the --domain or --scope flag (for example: lark-cli auth login
--domain <your-domain> or --scope <scope>) so the auth flow accepts the request,
referencing the same auth flow rules from lark-shared/SKILL.md and the "lark-cli
auth login" command in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa60a323-b133-4e3f-8f49-71dc4fbef926
📒 Files selected for processing (1)
skills/lark-sheets/SKILL.md
Summary
Context
This PR only addresses issues that still exist on the latest main branch. The older review notes about scope tables and resource-relationship duplication no longer apply to current main, so they were intentionally left untouched.
Testing
Summary by CodeRabbit