feat(im): show bot sender display names when reading messages#1829
Conversation
📝 WalkthroughWalkthroughAdds server-side sender-name resolution to Lark IM shortcuts: query parameters request ChangesSender name resolution feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Command as IM Shortcut Command
participant LarkAPI as Lark Server API
participant ResolveSenderNames
participant ContactAPI as Contact API
participant senderDisplay
Command->>LarkAPI: request messages with with_sender_name=true
LarkAPI-->>Command: messages with sender_name/sender_i18n_names
Command->>ResolveSenderNames: harvest sender names from messages
ResolveSenderNames->>ResolveSenderNames: pickSenderName (producer names)
alt name missing for user sender
ResolveSenderNames->>ContactAPI: batch resolve remaining ou_ ids
ContactAPI-->>ResolveSenderNames: resolved names
end
ResolveSenderNames-->>Command: nameMap
Command->>senderDisplay: render sender (name or id fallback)
senderDisplay-->>Command: display string
Command-->>Command: render output row
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/im/im_messages_mget.go (1)
114-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the sender-row logic into a shared helper.
This exact block (
sendertype-assert →senderDisplay→ conditionalrow["sender"]assignment) is duplicated verbatim inim_messages_search.go(lines 220-224) andim_threads_messages_list.go(lines 144-148). Consider adding a small helper alongsidesenderDisplay(e.g.attachSenderRow(row, msg)) insender_display.goand calling it from all three sites to avoid triplicated logic.♻️ Proposed helper (add to sender_display.go, then call from each shortcut)
// attachSenderRow sets row["sender"] from msg's sender field using senderDisplay, // leaving it unset when neither a name nor an id is available. func attachSenderRow(row, msg map[string]interface{}) { if sender, ok := msg["sender"].(map[string]interface{}); ok { if disp := senderDisplay(sender); disp != "" { row["sender"] = disp } } }- if sender, ok := msg["sender"].(map[string]interface{}); ok { - if disp := senderDisplay(sender); disp != "" { - row["sender"] = disp - } - } + attachSenderRow(row, msg)🤖 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/im/im_messages_mget.go` around lines 114 - 118, The sender-to-row mapping block is duplicated across multiple IM shortcut files, so extract it into a shared helper near senderDisplay. Add a small helper such as attachSenderRow(row, msg) in sender_display.go that performs the sender type assertion, calls senderDisplay, and conditionally sets row["sender"], then replace the repeated inline block in im_messages_mget.go, im_messages_search.go, and im_threads_messages_list.go with calls to that helper.
🤖 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.
Nitpick comments:
In `@shortcuts/im/im_messages_mget.go`:
- Around line 114-118: The sender-to-row mapping block is duplicated across
multiple IM shortcut files, so extract it into a shared helper near
senderDisplay. Add a small helper such as attachSenderRow(row, msg) in
sender_display.go that performs the sender type assertion, calls senderDisplay,
and conditionally sets row["sender"], then replace the repeated inline block in
im_messages_mget.go, im_messages_search.go, and im_threads_messages_list.go with
calls to that helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f4f23df-a31f-444b-a53c-f9c16c467600
📒 Files selected for processing (15)
CHANGELOG.mdshortcuts/im/builders_test.goshortcuts/im/convert_lib/helpers.goshortcuts/im/convert_lib/helpers_test.goshortcuts/im/coverage_additional_test.goshortcuts/im/helpers.goshortcuts/im/im_chat_messages_list.goshortcuts/im/im_messages_mget.goshortcuts/im/im_messages_search.goshortcuts/im/im_threads_messages_list.goshortcuts/im/sender_display.goshortcuts/im/sender_display_test.goshortcuts/im/with_sender_name_test.goskill-template/domains/im.mdskills/lark-im/SKILL.md
d50c988 to
227ab62
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@f5e47c7ad5a63bb2b91181933de67db74edb8893🧩 Skill updatenpx skills add sammi-bytedance/larksuite-cli#feat/im-bot-sender-name -y -g |
Read the server-provided sender_name for both user and bot senders (previously only users resolved) so message-read commands display bot names instead of raw ids. The CLI opts into server-side name filling by sending with_sender_name=true on chat-messages-list, threads-messages-list, messages-mget and messages-search, as well as on the inline fetches that render nested senders: merge_forward sub-messages and auto-expanded thread replies. Without it those nested-only senders carry no sender_name and, with no fallback, render as raw ids. Names come solely from the server (single source of truth): there is no contact or mention fallback, so the contact scope is dropped from these four commands and the contact/mention resolution code is removed. A sender the server does not name falls back to its id; system messages show no name. The resolved name is exposed in the existing `name` field (backward compatible); the duplicate raw `sender_name` is stripped while the full `sender_i18n_names` map and `open_bot_id` are preserved for consumers. No new permission scope is required. Updates the lark-im skill docs.
227ab62 to
f5e47c7
Compare
Summary
When reading messages, the CLI now shows a display name for bot senders as well as users (previously only users resolved). Names come solely from the server: the CLI opts in with
with_sender_name=trueand reads the returnedsender_name/sender_i18n_namesfor both users and bots, withopen_bot_idpassthrough and a graceful fallback to the sender id. There is no contact/mention fallback and no new permission scope.Changes
convert_lib: read the server-providedsender_name/sender_i18n_namesfor both user and bot senders (pickSenderName, i18n by locale);open_bot_idpasses through the whole sender object. The previous contact-batch and mention-based resolution is removed — a sender the server does not name falls back to its id.with_sender_name=trueopt-in: added tochat-messages-list/messages-mget/messages-search/threads-messages-list, and to the inline fetches that render nested senders — merge_forward sub-messages and auto-expanded thread replies — so senders that appear only inside those nested views also get named.senderDisplayhelper). System messages (msg_type: system) show no sender name, as expected.lark-imskill (template + generated) and add aCHANGELOGentry.Test Plan
go test ./shortcuts/im ./shortcuts/im/convert_lib(user+bot display, id fallback, system messages, i18n selection,open_bot_idpassthrough,with_sender_nameon all four commands + merge_forward / thread-reply inline fetches)go build ./.../go vet/gofmtcleanRelated Issues
Summary by CodeRabbit