feat: surface reply context and mentions in im.message.receive_v1#1798
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR expands IM message receive handling with sender type, thread/reply context, update time, and structured mentions. It updates compact event processing, shared mention conversion helpers, resolved-schema assertions, and receive-event tests. ChangesIM message metadata enrichment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Event as im.message.receive_v1
participant Receive as processImMessageReceive
participant Mentions as compactMentions
participant Output as ImMessageReceiveOutput
Event->>Receive: Parse message, sender, and relationship fields
Receive->>Mentions: Convert raw mentions
Mentions-->>Receive: Return compact mention entries
Receive-->>Output: Emit metadata, content, and mentions
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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)
events/im/message_receive.go (1)
114-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting shared mention helpers to avoid duplication.
The
compactMentions,stringField, andmentionOpenIDhelpers defined here appear to be duplicated inshortcuts/event/helpers.goper the PR stack context. Since these are unexported inevents/im, theshortcuts/eventpackage can't import them, but a shared internal package (e.g.,internal/mentionor similar) would eliminate the copy and ensure both call sites stay in sync.This is deferred-able if there's a circular-dependency concern or if the two packages intentionally maintain separate copies.
🤖 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 `@events/im/message_receive.go` around lines 114 - 151, The mention-compaction helpers are duplicated across packages, so move the shared logic from compactMentions, stringField, and mentionOpenID into a common internal helper package that both events/im/message_receive.go and shortcuts/event can use. Keep the existing behavior the same, but have the local call sites delegate to the shared helper functions so future changes stay in sync and the duplication is removed.
🤖 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 `@events/im/message_receive.go`:
- Around line 114-151: The mention-compaction helpers are duplicated across
packages, so move the shared logic from compactMentions, stringField, and
mentionOpenID into a common internal helper package that both
events/im/message_receive.go and shortcuts/event can use. Keep the existing
behavior the same, but have the local call sites delegate to the shared helper
functions so future changes stay in sync and the duplication is removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 783f42ae-93ea-4266-aea1-8cec8fbbc002
📒 Files selected for processing (6)
cmd/event/schema_test.goevents/im/message_receive.goevents/im/message_receive_test.goshortcuts/event/helpers.goshortcuts/event/processor_im_message.goshortcuts/event/processor_test.go
💤 Files with no reviewable changes (1)
- shortcuts/event/processor_test.go
ab90667 to
b11ca86
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@db6f851e41bfc7bd1f5f4b8748a70903700e94f7🧩 Skill updatenpx skills add 91-enjoy/cli#feat/im-message-receive-agent-fields -y -g |
b11ca86 to
3e132eb
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/event/helpers.go (1)
37-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHelpers are correct and nil-safe; consider sharing duplicated utilities.
All three functions handle nil maps and unexpected types gracefully — failed type assertions produce zero values that flow through the empty-string guards correctly. The conditional field inclusion and nil-on-empty return contract in
compactMentionsmatches the downstream consumer's expectations (key/id/name per context snippet 2).
stringFieldandmentionOpenIDare byte-for-byte identical to the same-named functions inevents/im/message_receive.go(context snippet 1). If both packages will continue evolving these mention-extraction rules together, extracting the two pure helpers into a shared internal package would prevent silent divergence. For now this is acceptable given the small surface area and different package boundaries.Per coding guidelines, every behavior change needs a test alongside the change. These helpers are likely exercised indirectly via
processor_test.go, but dedicated unit tests formentionOpenID's three branches (nested map, plain string, unsupported type) andcompactMentions's nil/empty paths would lock in the contract at the helper level.🤖 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/event/helpers.go` around lines 37 - 83, Keep the existing helper behavior, but add dedicated unit tests for mentionOpenID covering nested-map, plain-string, and unsupported-type inputs, and for compactMentions covering nil/empty inputs and its nil-on-empty result contract. Place the tests alongside these helpers and avoid changing production logic or duplicating shared utilities.Source: Coding guidelines
🤖 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/event/helpers.go`:
- Around line 37-83: Keep the existing helper behavior, but add dedicated unit
tests for mentionOpenID covering nested-map, plain-string, and unsupported-type
inputs, and for compactMentions covering nil/empty inputs and its nil-on-empty
result contract. Place the tests alongside these helpers and avoid changing
production logic or duplicating shared utilities.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a06cf34d-75de-47d4-96cf-843928a5a014
📒 Files selected for processing (6)
cmd/event/schema_test.goevents/im/message_receive.goevents/im/message_receive_test.goshortcuts/event/helpers.goshortcuts/event/processor_im_message.goshortcuts/event/processor_test.go
💤 Files with no reviewable changes (1)
- shortcuts/event/processor_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- cmd/event/schema_test.go
- events/im/message_receive_test.go
- events/im/message_receive.go
- shortcuts/event/processor_im_message.go
b6edba4 to
30881f2
Compare
30881f2 to
db6f851
Compare
Summary
This PR improves the
im.message.receive_v1event output by exposing structuralmetadata fields (reply context, sender type, mentions) that were previously only
available in the raw V2 envelope. It also syncs the same structural fields to the legacy
+subscribe --compactpipeline.Changes
1. Event output enrichment (
events/im/message_receive.go)The
ImMessageReceiveOutputstruct is extended with seven new fields:sender_typeevent.sender.sender_typeuserorbotroot_idevent.message.root_idthread_idevent.message.thread_idreply_toevent.message.parent_idmentionsevent.message.mentionskey,id,nameupdate_timeevent.message.update_timeevent_idheader.event_idmessage_idfor dedup)The redundant
parent_idfield is removed;reply_toserves as the canonicalparent message reference, aligned with
im +messages-mget.Helper functions
compactMentions,stringField, andmentionOpenIDare addedfor safe mention extraction from the raw mentions array.
2. Old compact pipeline sync (
shortcuts/event/)The
ImMessageProcessor.Transformcompact mode for+subscribe --compactisupdated with the same seven fields (
event_id,sender_type,root_id,thread_id,reply_to,mentions,update_time). Existing behavior ispreserved — the
interactivefallback to raw is unchanged, and the originaltimestamp fallback logic (header → message create_time) is retained.
Shared helpers (
stringField,mentionOpenID,compactMentions) are added toshortcuts/event/helpers.gofor reuse across processors.Test Plan
Event output
go test ./events/im/...—TestProcessImMessageReceive_Textverifiesall new fields (sender_type, root_id, thread_id, reply_to, mentions,
update_time) are present and correctly populated from raw payload.
TestProcessImMessageReceive_OmitsUnchangedUpdateTimeverifiesupdate_time is omitted when it equals create_time.
go test ./cmd/event/...—TestRunSchema_ReceiveMessageAgentFieldsJSONverifies the reflected JSON schema includes root_id, thread_id, reply_to,
sender_type, and mentions; confirms no parent_id.
Compact pipeline
go test ./shortcuts/event/...— all existing tests pass, includingTestImMessageProcessor_CompactInteractiveFallsBackToRaw(unchanged behavior).Related Issues
None
Summary by CodeRabbit
Summary
New Features
im.message.receive_v1output with reply/thread context,sender_type, conditionalupdate_time, and structured mentions (key,id,name).Bug Fixes
update_timeis omitted when it matchescreate_time.Tests
im.message.receive_v1resolved output fields.