feat(vc): refine meeting-events output and reaction forwarding#1674
Merged
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:
📝 WalkthroughWalkthroughAdds bot VC event handling and registration, refactors ChangesBot-observed VC EventKeys
meeting-events shortcut identity/roster rework
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant EventBus
participant processVCBotEvent
participant fillBotEventOutput
EventBus->>processVCBotEvent: raw.Payload
processVCBotEvent->>processVCBotEvent: decode envelope
alt decode fails
processVCBotEvent-->>EventBus: return raw.Payload unchanged
else decode succeeds
processVCBotEvent->>fillBotEventOutput: envelope, event type
fillBotEventOutput->>fillBotEventOutput: decode payload and trim fields
fillBotEventOutput-->>processVCBotEvent: populated VCBotEventOutput
processVCBotEvent-->>EventBus: marshaled JSON output
end
sequenceDiagram
participant CLI
participant Execute
participant Core
participant RosterAPI
participant TimelineBuilder
CLI->>Execute: run +meeting-events
Execute->>Core: resolve identity
Execute->>RosterAPI: fetch current roster
RosterAPI-->>Execute: roster or warning
Execute->>TimelineBuilder: build typed output
TimelineBuilder-->>Execute: events and metadata rows
Execute-->>CLI: json, ndjson, or pretty output
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 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 |
github-actions
Bot
added
domain/vc
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1674 +/- ##
==========================================
+ Coverage 74.39% 74.42% +0.02%
==========================================
Files 860 860
Lines 89481 89754 +273
==========================================
+ Hits 66571 66797 +226
- Misses 17750 17779 +29
- Partials 5160 5178 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Ren1104
force-pushed
the
features/F-vc-bot-meeting-activity-contract
branch
10 times, most recently
from
July 1, 2026 07:22
3d25d78 to
e72bb64
Compare
Ren1104
marked this pull request as ready for review
July 1, 2026 07:22
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b584df97bd5fd7ecf5a8fc533922a872cbc3e0df🧩 Skill updatenpx skills add larksuite/cli#features/F-vc-bot-meeting-activity-contract -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
events/vc/bot_events.go (3)
116-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate unmarshal wrappers — candidate for a generic helper.
decodeBotMeetingInvitedEvent,decodeBotMeetingActivityEvent, anddecodeBotMeetingEndedEventdiffer only by the target type. A small generic helper would remove the repetition.♻️ Suggested generic helper
-func decodeBotMeetingInvitedEvent(data json.RawMessage) (vcBotMeetingInvitedEvent, error) { - var payload vcBotMeetingInvitedEvent - if err := json.Unmarshal(data, &payload); err != nil { - return vcBotMeetingInvitedEvent{}, err - } - return payload, nil -} - -func decodeBotMeetingActivityEvent(data json.RawMessage) (vcBotMeetingActivityEvent, error) { - var payload vcBotMeetingActivityEvent - if err := json.Unmarshal(data, &payload); err != nil { - return vcBotMeetingActivityEvent{}, err - } - return payload, nil -} - -func decodeBotMeetingEndedEvent(data json.RawMessage) (vcBotMeetingEndedEvent, error) { - var payload vcBotMeetingEndedEvent - if err := json.Unmarshal(data, &payload); err != nil { - return vcBotMeetingEndedEvent{}, err - } - return payload, nil -} +func decodeBotEventPayload[T any](data json.RawMessage) (T, error) { + var payload T + err := json.Unmarshal(data, &payload) + return payload, err +}Call sites become
decodeBotEventPayload[vcBotMeetingInvitedEvent](data), etc.🤖 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/vc/bot_events.go` around lines 116 - 138, The three decodeBotMeeting*Event functions are duplicated json.Unmarshal wrappers that only differ by target type. Replace them with a shared generic helper such as decodeBotEventPayload[T] and update the existing decodeBotMeetingInvitedEvent, decodeBotMeetingActivityEvent, and decodeBotMeetingEndedEvent call sites to use it so the payload decoding logic lives in one place.
26-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThree wrapper functions are functionally identical.
processVCBotMeetingInvited,processVCBotMeetingEvent, andprocessVCBotMeetingEndedall just forward toprocessVCBotEvent(raw), which determines the effective event type from the payload itself (not from which wrapper was called). Consider collapsing these into a single shared adapter referenced three times inregister.go, or add a short comment explaining why three distinct identifiers are kept (e.g., for registry introspection/testing).♻️ Optional consolidation
-func processVCBotMeetingInvited(_ context.Context, _ event.APIClient, raw *event.RawEvent, _ map[string]string) (json.RawMessage, error) { - return processVCBotEvent(raw) -} - -func processVCBotMeetingEvent(_ context.Context, _ event.APIClient, raw *event.RawEvent, _ map[string]string) (json.RawMessage, error) { - return processVCBotEvent(raw) -} - -func processVCBotMeetingEnded(_ context.Context, _ event.APIClient, raw *event.RawEvent, _ map[string]string) (json.RawMessage, error) { - return processVCBotEvent(raw) -} +func processVCBotEventEnvelope(_ context.Context, _ event.APIClient, raw *event.RawEvent, _ map[string]string) (json.RawMessage, error) { + return processVCBotEvent(raw) +}Then reference
processVCBotEventEnvelopefor all threeProcessfields inregister.go.🤖 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/vc/bot_events.go` around lines 26 - 36, The three wrapper functions in bot_events.go are identical pass-throughs to processVCBotEvent, so consolidate the duplication or document why separate identifiers are required. Update the shared adapter approach by introducing a single wrapper like processVCBotEventEnvelope and reference that same function from the three Process registrations in register.go, or keep the three names only if there is a clear registry/testing reason explained in a comment.
71-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the no-op
decoder.UseNumber()call.It only changes decoding for
interface{}targets;vcBotEventEnvelopeuses typed fields andjson.RawMessage, so this line has no effect.🤖 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/vc/bot_events.go` around lines 71 - 73, Remove the no-op json.Decoder.UseNumber call in vcBotEventEnvelope decoding. The decoder in bot_events.go is only used by the event parsing path around decoder.Decode(&envelope), and since vcBotEventEnvelope contains typed fields plus json.RawMessage, UseNumber has no effect here. Delete that call and keep the decode flow otherwise unchanged.events/vc/register.go (1)
21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConstant name doesn't match its event/DisplayName ("activity").
eventTypeBotMeetingEventbacks"vc.bot.meeting_activity_v1"/ DisplayName "Bot meeting activity", but the identifier reads as a generic "meeting event" rather than "meeting activity".eventTypeBotMeetingActivitywould be clearer and match the pattern of the other two constants (...Invited,...Ended).🤖 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/vc/register.go` around lines 21 - 23, The constant name in the vc register event types is inconsistent with its actual event and display name, since eventTypeBotMeetingEvent represents the meeting activity event. Rename eventTypeBotMeetingEvent to eventTypeBotMeetingActivity and update any references in register.go or related registration logic so the identifier matches the "vc.bot.meeting_activity_v1" / "Bot meeting activity" naming pattern.events/vc/bot_events_test.go (1)
235-277: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor test-coverage gap: only
meeting_activityis tested for nested malformed payload.
TestProcessVCBotMeetingEvent_MalformedActivityPayloadKeepsRawEventonly covers the activity branch offillBotEventOutput. SincedecodeBotMeetingInvitedEvent/decodeBotMeetingEndedEventshare the same early-return-on-error pattern, an analogous case for invited/ended would round out coverage, though the shared code path already gives reasonable confidence.🤖 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/vc/bot_events_test.go` around lines 235 - 277, Add a companion test for the same malformed nested-payload behavior in the other `fillBotEventOutput` branches, not just `TestProcessVCBotMeetingEvent_MalformedActivityPayloadKeepsRawEvent`. Create a case that exercises `decodeBotMeetingInvitedEvent` or `decodeBotMeetingEndedEvent` with invalid `meeting_invited_items`/`meeting_ended_items`, and assert the stable fields stay empty while `RawEvent` is preserved, mirroring `runBotEventProcess` and the existing malformed activity test.shortcuts/vc/vc_meeting_events.go (1)
158-165: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift
rolefield is overloaded with two different meanings acrossidentityvscurrent_roster/actors.For the top-level
identityobject,Roleis a static auth-mode tag ("user"/"bot"), but forcurrent_roster[]/eventactors[](built viameetingEventsIdentityFromParticipant), the same JSON fieldrolecarries meeting-role semantics ("host"/"co_host"/"participant"/"bot"). A consuming agent inspectingrolegenerically (e.g., to find the meeting host) could be confused when the same person's self-entry showsrole:"user"at the top level butrole:"host"insidecurrent_roster. Consider distinguishing these (e.g.,auth_rolefor the top-level identity, or deriving the real meeting role foridentitytoo by cross-referencing the fetched roster) before this contract is more widely consumed by agents.Also applies to: 337-361
🤖 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/vc/vc_meeting_events.go` around lines 158 - 165, The meeting identity payload is using the same role field for two different meanings, which can confuse consumers across meetingEventsIdentity and meetingEventsIdentityFromParticipant. Update the top-level identity contract to use a distinct auth-specific field (for example via meetingEventsIdentity) or otherwise derive the actual meeting role consistently from the roster so identity, current_roster, and actors do not conflict. Make sure the JSON shape and any builder logic in meetingEventsIdentityFromParticipant align with the chosen naming/semantics.
🤖 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/vc/vc_meeting_events.go`:
- Around line 188-199: The meeting snapshot in `meetingEventsCurrentRoster`
handling is being taken from the first event only, which can leave
`output.Meeting` with stale `status` and `end_time`. Update the loop in
`vc_meeting_events.go` so the meeting snapshot is derived from the latest
available event payload rather than only when `output.Meeting.ID` is empty, and
make sure `meetingEventsMeetingFromPayload` still runs when needed so the
empty-events case produces the expected default meeting state. Keep
`output.Events` behavior unchanged while ensuring `output.Meeting` reflects the
newest snapshot.
In `@skills/lark-vc-agent/references/lark-vc-agent-meeting-events.md`:
- Around line 287-289: The `20001 meeting_status_MEETING_END` guidance uses the
wrong shortcut name for fetching meeting artifacts; it should match the rest of
this skill and `lark-vc-agent/SKILL.md`. Update the `lark-cli vc +notes
--meeting-ids <meeting.id>` reference in this row to the same shortcut used
elsewhere (`+detail`), and keep the rest of the branching logic on
`note_display_type`, `note_id`, and `minute_token` unchanged.
---
Nitpick comments:
In `@events/vc/bot_events_test.go`:
- Around line 235-277: Add a companion test for the same malformed
nested-payload behavior in the other `fillBotEventOutput` branches, not just
`TestProcessVCBotMeetingEvent_MalformedActivityPayloadKeepsRawEvent`. Create a
case that exercises `decodeBotMeetingInvitedEvent` or
`decodeBotMeetingEndedEvent` with invalid
`meeting_invited_items`/`meeting_ended_items`, and assert the stable fields stay
empty while `RawEvent` is preserved, mirroring `runBotEventProcess` and the
existing malformed activity test.
In `@events/vc/bot_events.go`:
- Around line 116-138: The three decodeBotMeeting*Event functions are duplicated
json.Unmarshal wrappers that only differ by target type. Replace them with a
shared generic helper such as decodeBotEventPayload[T] and update the existing
decodeBotMeetingInvitedEvent, decodeBotMeetingActivityEvent, and
decodeBotMeetingEndedEvent call sites to use it so the payload decoding logic
lives in one place.
- Around line 26-36: The three wrapper functions in bot_events.go are identical
pass-throughs to processVCBotEvent, so consolidate the duplication or document
why separate identifiers are required. Update the shared adapter approach by
introducing a single wrapper like processVCBotEventEnvelope and reference that
same function from the three Process registrations in register.go, or keep the
three names only if there is a clear registry/testing reason explained in a
comment.
- Around line 71-73: Remove the no-op json.Decoder.UseNumber call in
vcBotEventEnvelope decoding. The decoder in bot_events.go is only used by the
event parsing path around decoder.Decode(&envelope), and since
vcBotEventEnvelope contains typed fields plus json.RawMessage, UseNumber has no
effect here. Delete that call and keep the decode flow otherwise unchanged.
In `@events/vc/register.go`:
- Around line 21-23: The constant name in the vc register event types is
inconsistent with its actual event and display name, since
eventTypeBotMeetingEvent represents the meeting activity event. Rename
eventTypeBotMeetingEvent to eventTypeBotMeetingActivity and update any
references in register.go or related registration logic so the identifier
matches the "vc.bot.meeting_activity_v1" / "Bot meeting activity" naming
pattern.
In `@shortcuts/vc/vc_meeting_events.go`:
- Around line 158-165: The meeting identity payload is using the same role field
for two different meanings, which can confuse consumers across
meetingEventsIdentity and meetingEventsIdentityFromParticipant. Update the
top-level identity contract to use a distinct auth-specific field (for example
via meetingEventsIdentity) or otherwise derive the actual meeting role
consistently from the roster so identity, current_roster, and actors do not
conflict. Make sure the JSON shape and any builder logic in
meetingEventsIdentityFromParticipant align with the chosen naming/semantics.
🪄 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: de1ead99-be64-4d3c-b72d-1270e7592e16
📒 Files selected for processing (10)
events/vc/bot_events.goevents/vc/bot_events_test.goevents/vc/register.goshortcuts/vc/vc_meeting_events.goshortcuts/vc/vc_meeting_events_test.goskills/lark-event/SKILL.mdskills/lark-event/references/lark-event-vc.mdskills/lark-vc-agent/SKILL.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-events.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/vc/vc_meeting_events_test.go (1)
553-1005: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated stub-registration boilerplate across tests.
Nearly every test in this range repeats the same three-stub setup (
meetingEventsStub+botInfoStub()+meetingDetailParticipantsStub(...)) with--as bot. A small helper (e.g.registerBotModeStubs(reg, events, hasMore, pageToken, participants)) would reduce duplication without hurting test readability.🤖 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/vc/vc_meeting_events_test.go` around lines 553 - 1005, The test setup is duplicated across many VCMeetingEvents cases, especially the repeated `meetingEventsStub`, `botInfoStub`, and `meetingDetailParticipantsStub` registration for `--as bot`. Add a small helper in `vc_meeting_events_test.go` (for example, a registration helper used by `TestMeetingEvents_ExecuteJSON`, `TestMeetingEvents_ExecutePretty`, and related tests) that takes the event list, pagination flags, and participants and registers the stubs in one place. Update the affected tests to call that helper so the repeated boilerplate is removed while keeping each test’s assertions focused.
🤖 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/vc/vc_meeting_events_test.go`:
- Around line 553-1005: The test setup is duplicated across many VCMeetingEvents
cases, especially the repeated `meetingEventsStub`, `botInfoStub`, and
`meetingDetailParticipantsStub` registration for `--as bot`. Add a small helper
in `vc_meeting_events_test.go` (for example, a registration helper used by
`TestMeetingEvents_ExecuteJSON`, `TestMeetingEvents_ExecutePretty`, and related
tests) that takes the event list, pagination flags, and participants and
registers the stubs in one place. Update the affected tests to call that helper
so the repeated boilerplate is removed while keeping each test’s assertions
focused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ebb67d0-fa45-4b45-861c-7b6d9dafe4b3
📒 Files selected for processing (4)
shortcuts/vc/vc_meeting_events.goshortcuts/vc/vc_meeting_events_test.goskills/lark-vc-agent/SKILL.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-events.md
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/vc/vc_meeting_events.go
Ren1104
force-pushed
the
features/F-vc-bot-meeting-activity-contract
branch
from
July 6, 2026 08:44
ee0ed99 to
74090fe
Compare
Ren1104
changed the title
Ren1104
changed the title
Ren1104
changed the title
Ren1104
changed the title
Ren1104
force-pushed
the
features/F-vc-bot-meeting-activity-contract
branch
from
July 7, 2026 12:36
69ce678 to
a36f63e
Compare
Ren1104
changed the title
Ren1104
force-pushed
the
features/F-vc-bot-meeting-activity-contract
branch
from
July 8, 2026 03:10
576d4bd to
b5f3e21
Compare
zhaoleibd
approved these changes
Jul 8, 2026
Ren1104
force-pushed
the
features/F-vc-bot-meeting-activity-contract
branch
from
July 8, 2026 07:08
b5f3e21 to
b584df9
Compare
This was referenced Jul 8, 2026
Merged
Closed
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
This PR refines the
vc +meeting-eventsoutput contract and updates the VC agent guidance for forwarding meeting chat/reactions to IM.Changes include:
vc +meeting-eventsoutput around stable top-levelmeeting,identity,events,warnings,has_more, andpage_tokenfieldsevent_id,event_type,event_time, normalizedactors, and event-specificpayloadfor participant, chat/reaction, transcript, and magic-share eventsleave_reasonindicates the meeting endedlark-vc-agentguidance so agents use JSON events for IM forwarding and only emit Feishu postemotionnodes for reaction keys supported by the IM reaction whitelist, falling back unknown reaction keys to textValidation
go test ./cmd ./shortcuts/vcgit diff --checkgofmt -l cmd/root_integration_test.go shortcuts/vc/vc_meeting_events.go shortcuts/vc/vc_meeting_events_test.go