feat(im): resolve bot sender names in shortcuts#848
Open
os-zzw wants to merge 2 commits into
Open
Conversation
github-actions
Bot
added
domain/im
|
|
|
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 (4)
📝 WalkthroughWalkthroughPre-resolves bot/app sender display names (from welcome HTML, bots/basic_batch, and runtime.BotInfo) before resolving users; adds the bot basic-info OAuth scope to IM shortcuts, tests bot/app resolution, attaches resolved sender names during feed-thread enrichment, and updates docs. ChangesBot/App Sender Resolution and Scope Declaration
Sequence Diagram(s)sequenceDiagram
participant Client
participant ResolveSenderNames
participant AppWelcomeExtractor
participant BotBasicBatchAPI
participant Runtime.BotInfo
participant ContactBatchAPI
Client->>ResolveSenderNames: provide messages with senders
ResolveSenderNames->>AppWelcomeExtractor: parse welcome-card HTML for app display names
AppWelcomeExtractor-->>ResolveSenderNames: update name cache for app senders
ResolveSenderNames->>BotBasicBatchAPI: batch request display names for bot/app IDs
BotBasicBatchAPI-->>ResolveSenderNames: return bot display names
ResolveSenderNames->>Runtime.BotInfo: fetch current bot metadata
Runtime.BotInfo-->>ResolveSenderNames: return current bot display name
ResolveSenderNames->>ContactBatchAPI: batch request user display names for remaining IDs
ContactBatchAPI-->>ResolveSenderNames: return user display names
ResolveSenderNames-->>Client: messages with resolved sender names
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/im/convert_lib/helpers.go (1)
158-158: 💤 Low valueLocale-specific regex pattern.
The regex
我是\s+\*\*([^*\n]+)\*\*is hardcoded for Chinese welcome messages and won't extract bot names from messages in other locales. This is acceptable since the batch API fallback (lines 282-308) will still resolve bot names when regex extraction fails. Consider documenting this limitation if welcome card formats vary by locale.🤖 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/convert_lib/helpers.go` at line 158, The regex stored in appWelcomeNameRe (var appWelcomeNameRe = regexp.MustCompile(`我是\s+\*\*([^*\n]+)\*\*`)) is Chinese‑locale specific; add a concise inline comment near appWelcomeNameRe noting this limitation and that bot name extraction will fail for other locales, and reference the fallback behavior that resolves names via the batch API (the existing fallback block) so future maintainers understand why the pattern is narrow and where name resolution occurs if the regex doesn't match.
🤖 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/im/convert_lib/helpers.go`:
- Around line 282-308: The batchResolveBots function calls a possibly incorrect
endpoint "/open-apis/bot/v3/bots/basic_batch" via runtime.DoAPIJSON; verify the
correct Lark API path or whether this is an internal/undocumented API and
replace the endpoint string and any query parameter keys (e.g., "bot_ids") with
the documented ones, or implement a safe fallback (per-ID lookup using the
documented bot info endpoint) and add proper error handling/logging around
runtime.DoAPIJSON to avoid silent failures; update only the endpoint/query usage
and error path in batchResolveBots to use the confirmed API or the fallback.
---
Nitpick comments:
In `@shortcuts/im/convert_lib/helpers.go`:
- Line 158: The regex stored in appWelcomeNameRe (var appWelcomeNameRe =
regexp.MustCompile(`我是\s+\*\*([^*\n]+)\*\*`)) is Chinese‑locale specific; add a
concise inline comment near appWelcomeNameRe noting this limitation and that bot
name extraction will fail for other locales, and reference the fallback behavior
that resolves names via the batch API (the existing fallback block) so future
maintainers understand why the pattern is narrow and where name resolution
occurs if the regex doesn't match.
🪄 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: ee247f48-381b-4d5d-89a3-18cf39cddfa1
📒 Files selected for processing (1)
shortcuts/im/convert_lib/helpers.go
os-zzw
force-pushed
the
feat_message_bot_name
branch
from
1a2f157 to
2c496b9
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #848 +/- ##
==========================================
+ Coverage 65.67% 65.92% +0.25%
==========================================
Files 513 518 +5
Lines 47655 48919 +1264
==========================================
+ Hits 31297 32250 +953
- Misses 13652 13896 +244
- Partials 2706 2773 +67 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@2c496b99fd2e30d29e53d049e685035dbe7b7196🧩 Skill updatenpx skills add os-zzw/cli#feat_message_bot_name -y -g |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enhance the bot sender name resolution for IM shortcuts and ensure the correct bot scope is applied.
Changes
/open-apis/bot/v3/bots/basic_batchAPI.BotInfo()without making extra API calls.UserScopesandBotScopesin related IM shortcuts to include the requiredapplication:bot.basic_info:readscope.Test Plan
lark-cli im +chat-messages-listto ensure bot names are displayed correctly.Related Issues
None
Summary by CodeRabbit
New Features
Updates
Tests
Documentation