Skip to content

docs: clarify mail message shortcut guidance#1306

Open
bubbmon233 wants to merge 2 commits into
larksuite:mainfrom
bubbmon233:feat/89883ef
Open

docs: clarify mail message shortcut guidance#1306
bubbmon233 wants to merge 2 commits into
larksuite:mainfrom
bubbmon233:feat/89883ef

Conversation

@bubbmon233
Copy link
Copy Markdown
Collaborator

@bubbmon233 bubbmon233 commented Jun 6, 2026

Generated by the harness-coding skill.

  • Branch: feat/89883ef
  • Target: main

Sprints

ID Title Status Commit
S1 Update mail shortcut help, docs, triage tip, and tests passed 9542b07
S2 Synthesize transport contract for larksuite/cli passed 5788a6c

Source specs


This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.

Summary by CodeRabbit

  • Documentation

    • Clarified mail +message is single-message only; added guidance to use mail +messages for multiple IDs, client-side batching (20 IDs/request), merge behavior, and backend raw limit (50); updated triage/skill/reference docs and usage tips.
  • Tests

    • Added/updated tests covering help text, dry-run wording, single vs. batch usage, chunking, merging, and backend-limit behavior.
  • Improvements

    • Switched several IM feed list/query fetches to typed JSON decoding for more robust response handling.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 6, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 6, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 98e3c03c-0bf6-4c6e-a6a0-d7fbfc49061b

📥 Commits

Reviewing files that changed from the base of the PR and between bcf7bf1 and fcba9da.

📒 Files selected for processing (14)
  • shortcuts/im/im_feed_group_list.go
  • shortcuts/im/im_feed_group_list_item.go
  • shortcuts/im/im_feed_group_query_item.go
  • shortcuts/mail/mail_message.go
  • shortcuts/mail/mail_message_help_test.go
  • shortcuts/mail/mail_messages.go
  • shortcuts/mail/mail_messages_test.go
  • shortcuts/mail/mail_triage.go
  • shortcuts/mail/mail_triage_test.go
  • skill-template/domains/mail.md
  • skills/lark-mail/SKILL.md
  • skills/lark-mail/references/lark-mail-message.md
  • skills/lark-mail/references/lark-mail-messages.md
  • skills/lark-mail/references/lark-mail-triage.md
✅ Files skipped from review due to trivial changes (5)
  • shortcuts/mail/mail_messages.go
  • skills/lark-mail/references/lark-mail-message.md
  • skills/lark-mail/SKILL.md
  • shortcuts/mail/mail_message.go
  • shortcuts/mail/mail_triage.go
🚧 Files skipped from review as they are similar to previous changes (8)
  • shortcuts/mail/mail_triage_test.go
  • shortcuts/im/im_feed_group_query_item.go
  • shortcuts/im/im_feed_group_list.go
  • shortcuts/mail/mail_messages_test.go
  • skill-template/domains/mail.md
  • skills/lark-mail/references/lark-mail-messages.md
  • skills/lark-mail/references/lark-mail-triage.md
  • shortcuts/mail/mail_message_help_test.go

📝 Walkthrough

Walkthrough

Clarifies mail shortcuts: mail +message is single-ID only; mail +messages auto-chunks at 20 IDs per backend request, merges outputs, and backend rejects >50 IDs. Updates help/dry-run tests, triage tips, skills/docs, and switches several IM feed-group shortcuts to typed JSON decoding.

Changes

Mail shortcut guidance and documentation

Layer / File(s) Summary
Single-message shortcut clarification
shortcuts/mail/mail_message.go, shortcuts/mail/mail_message_help_test.go, skills/lark-mail/references/lark-mail-message.md
mail +message metadata and DryRun now require a single --message-id, direct multi-ID use to mail +messages, and add help/dry-run tests and reference updates.
Multi-message shortcut batching
shortcuts/mail/mail_messages.go, shortcuts/mail/mail_messages_test.go, shortcuts/mail/mail_message_help_test.go, skills/lark-mail/references/lark-mail-messages.md
mail +messages metadata and DryRun document CLI auto-chunking at 20 IDs per batch_get, ordered merging of responses, collection of unavailable IDs, and backend raw validation rejecting >50 IDs; tests updated to validate chunked requests and merged output (41-IDs case).
Triage pagination and follow-up guidance
shortcuts/mail/mail_triage.go, shortcuts/mail/mail_triage_test.go, skills/lark-mail/references/lark-mail-triage.md
Triage "next page" tip now shows both single-message (mail +message --message-id) and multi-message (mail +messages --message-ids) follow-ups (includes --mailbox when applicable); tests assert both variants and the absence of the prior single-only tip.
Skill & domain documentation
skill-template/domains/mail.md, skills/lark-mail/SKILL.md, skills/lark-mail/references/*
Add enforceable constraints requiring destructive/state-changing targets come from real query outputs; document +message/+messages/+thread usage, anti-looping guidance, CLI batching rules, and examples (e.g., +messages --html=false).

IM typed response updates

Layer / File(s) Summary
+feed-group-list typed decoding
shortcuts/im/im_feed_group_list.go
Switch non-pagination and pagination API calls to DoAPIJSONTyped to decode typed group responses.
+feed-group-list-item typed decoding
shortcuts/im/im_feed_group_list_item.go
Use DoAPIJSONTyped in Execute and --page-all per-page fetch paths; enrichment and merge logic unchanged.
+feed-group-query-item typed decoding
shortcuts/im/im_feed_group_query_item.go
Use runtime.DoAPIJSONTyped for the execute-path POST response decoding.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CLI as "mail +messages"
  participant Backend as "messages.batch_get"
  participant CLI_Merge as "CLI merger"
  User->>CLI: provide N message IDs (e.g., 41)
  CLI->>Backend: request IDs[0:20]
  Backend-->>CLI: messages 0-19
  CLI->>Backend: request IDs[20:40]
  Backend-->>CLI: messages 20-39
  CLI->>Backend: request IDs[40:41]
  Backend-->>CLI: message 40
  CLI->>CLI_Merge: merge responses preserving input order, collect unavailable IDs
  CLI_Merge-->>User: merged output for all IDs (or backend rejects if total > 50)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • larksuite/cli#1238: Modifies the same mail_triage.go / mail_triage_test.go area around the post-table tip and mailbox context.
  • larksuite/cli#1202: Related to mail +messages multi-ID flow and CLI-side batching/validation.
  • larksuite/cli#301: Overlaps on triage paging-tip changes and next-page hint behavior.

Suggested labels

documentation

Suggested reviewers

  • chanthuang
  • infeng

Poem

🐰 One message, two messages—choose your fetch with care,
Single ID hops, or batch them in a pair.
Twenty per call, then merge them in line,
Tests and docs snug so the CLI behaves fine.
Hop, fetch, and read—no looping, just shine!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description does not follow the required template. It lacks a proper Summary, Changes list, Test Plan section, and Related Issues section as specified in the template. Provide a description following the template: include a brief summary (1-3 sentences), list main changes, describe test verification, and document related issues or tests.
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'docs: clarify mail message shortcut guidance' accurately describes the main change: documentation updates to clarify usage guidance for mail message shortcuts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat/89883ef

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added domain/mail PR touches the mail domain size/M Single-domain feat or fix with limited business impact labels Jun 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 6, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9542b07307cb5e7be1306a9b29b9216029c9f6f6

🧩 Skill update

npx skills add bubbmon233/cli#feat/89883ef -y -g

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.98%. Comparing base (5788a6c) to head (9542b07).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1306      +/-   ##
==========================================
+ Coverage   70.95%   70.98%   +0.02%     
==========================================
  Files         685      685              
  Lines       65702    65702              
==========================================
+ Hits        46622    46639      +17     
+ Misses      15401    15383      -18     
- Partials     3679     3680       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added domain/im PR touches the im domain size/L Large or sensitive change across domains or core paths and removed size/M Single-domain feat or fix with limited business impact labels Jun 7, 2026
@bubbmon233 bubbmon233 force-pushed the feat/89883ef branch 2 times, most recently from bcf7bf1 to 214b2f3 Compare June 7, 2026 11:04
Clarify mail +message as single-email only and mail +messages as the multi-email path that chunks batch_get requests at 20 IDs and merges output while documenting the backend raw 50-ID validation limit.

Update the mail +triage table tip and lark-mail docs to route one selected message to +message and multiple selected messages to +messages.

Test: go test -count=1 ./shortcuts/mail
Change-Type: ci-fix
@@ -52,7 +52,7 @@ var MailMessages = common.Shortcut{
body["message_ids"] = messageIDs
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Review | [P2 正确性] dry-run 展示的 batch_get 请求与真实分批执行不一致

这里会把用户传入的全部 message_ids 放进一个 POST /messages/batch_get body,但 Execute 实际会按 20 个 ID 拆批。故障场景是 AI 对 41 或 60 个 ID 先跑 --dry-run:输出看起来像一次 raw 请求携带 41/60 个 ID,其中 60 个 ID 又与下方文案“raw request rejects more than 50 IDs”冲突,容易让调用方误判 +messages 不能处理 50+ 输入或复制出不可执行的 raw API 样例。

修复建议: 让 dry-run 也表达真实执行计划:对 >20 IDs 输出分批计划/多段 body,或只展示首批并明确 “dry-run body is illustrative; execution splits N IDs into M requests”,避免生成 raw-invalid 的单次请求。

如有疑问或认为判断不准确,欢迎直接回复讨论。

}

data, err := runtime.DoAPIJSON("GET", feedGroupListPath, feedGroupListGroupsQuery(runtime), nil)
data, err := runtime.DoAPIJSONTyped("GET", feedGroupListPath, feedGroupListGroupsQuery(runtime), nil)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Review | [P3 可维护性] IM feed group 错误契约迁移缺少回归覆盖

这里把 feed group 调用从 DoAPIJSON 切到 DoAPIJSONTyped,成功数据结构不变,但错误分类/输出契约会从 legacy API error 路径变为 typed error 路径;同类改动还出现在 im_feed_group_list_item.goim_feed_group_query_item.go。如果脚本或 AI 依赖旧的错误 envelope/message,非零 API code 场景会发生可见行为变化,但当前新增测试主要覆盖 mail 文案和成功分批,没有锁住 feed group 的错误输出。

修复建议: 补一组 feed group 非零 API code / HTTP error 的单测,断言返回的 code、message、log_id/subtype 等关键字段;或者在 PR 描述中明确这是有意的 IM 错误契约迁移。

如有疑问或认为判断不准确,欢迎直接回复讨论。

@bubbmon233
Copy link
Copy Markdown
Collaborator Author

🤖 AI Review | CR 汇总 | 有风险(1 个 P2,1 个 P3)

增量审查:基于已有自动审查评论,本次新增 2 条评论。

  • P2 正确性:mail +messages --dry-run 展示的单次 batch_get body 与真实 20 个 ID 分批执行不一致,>50 IDs 时还会呈现 raw-invalid 请求形态。
  • P3 可维护性:IM feed group 切到 typed API error 路径属于可见错误契约变化,建议补非零 API code / HTTP error 回归测试或在 PR 描述中明确范围。

已验证:go test -count=1 ./shortcuts/mailgo test -count=1 ./shortcuts/im -run 'FeedGroup|Feed' 通过。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/im PR touches the im domain domain/mail PR touches the mail domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants