feat(vc): extract note doc tokens from calendar event relation API#333
Merged
feat(vc): extract note doc tokens from calendar event relation API#333
Conversation
github-actions
Bot
added
domain/vc
|
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)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughResolve calendar→meeting logic now returns structured relation info (meeting IDs + meeting_notes). Note-fetching seeds from relation-info, attempts meeting-chain fetches, merges first successful meeting result, deduplicates overlapping tokens, and may fall back to relation-info tokens when meeting-chain fails. Recording call sites updated to consume the new relation-info. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CalendarAPI as Calendar API\n(mget_instance_relation_info)
participant MeetingAPI as Meeting API\n(fetch by meeting_id)
participant NoteAPI as Note Details API\n(get_note_detail)
Client->>CalendarAPI: mget_instance_relation_info(event_id, need_meeting_notes=true)
CalendarAPI-->>Client: eventRelationInfo { MeetingIDs, meeting_notes[] }
alt relation-info contains note tokens
Client->>NoteAPI: (optional) fetch details for relation-info tokens
NoteAPI-->>Client: note details
end
Client->>MeetingAPI: for each MeetingID -> fetch notes by meeting_id
MeetingAPI-->>Client: meeting note details (note_doc_token, verbatim_doc_token, shared_doc_tokens)
Client->>Client: merge relation-info tokens with meeting-detail tokens
Client->>Client: deduplicate meeting_notes against note_doc_token/verbatim/shared tokens
Client-->>Client: return merged, deduplicated note result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR extends the Confidence Score: 5/5Safe to merge — all changes are correct, well-tested, and non-breaking. No P0 or P1 findings. The deduplication logic is sound, the fallback path is correctly guarded, type handling through No files require special attention.
|
| Filename | Overview |
|---|---|
| shortcuts/vc/vc_notes.go | Core logic change: resolveMeetingIDsFromCalendarEvent returns *eventRelationInfo instead of []string, fetchNoteByCalendarEventID merges two token sources with deduplication via deduplicateDocTokens. Logic is correct; minor note that meeting_notes is always extracted from the API response even when needNotes=false. |
| shortcuts/vc/vc_notes_test.go | Seven new tests added covering extractStringSlice, deduplicateDocTokens (4 cases including all-duplicates), calendar-path dedup, fallback-on-chain-failure, and request-body verification; good coverage of the new code paths. |
| shortcuts/vc/vc_recording.go | Adapted to the new *eventRelationInfo return type; only relInfo.MeetingIDs is consumed, needNotes=false is correctly passed. No behavioral change. |
| shortcuts/vc/vc_recording_test.go | Existing recording tests updated to use the new relInfo.MeetingIDs accessor; all test logic is correct. |
| skills/lark-vc/SKILL.md | Documentation updated to describe the new meeting_notes field and its semantics (user-bound notes, calendar-event-ids path only); guidance is accurate and consistent with the implementation. |
| skills/lark-vc/references/lark-vc-notes.md | Reference doc updated to surface meeting_notes output field in examples and clarify its source; consistent with the code changes. |
Sequence Diagram
sequenceDiagram
participant CLI
participant CalAPI as Calendar API
participant VCAPI as VC API
CLI->>CalAPI: POST /calendars/primary
CalAPI-->>CLI: calendar_id
CLI->>CalAPI: POST /events/mget_instance_relation_info (need_meeting_notes=true)
CalAPI-->>CLI: meeting_instance_ids[] + meeting_notes[]
note over CLI: Source 1: meeting_notes tokens stored in result
loop for each meeting_id
CLI->>VCAPI: GET /meetings/{meeting_id}
VCAPI-->>CLI: note_id
CLI->>VCAPI: GET /notes/{note_id}
VCAPI-->>CLI: note_doc_token, verbatim_doc_token, shared_doc_tokens
note over CLI: Source 2: merge into result then deduplicateDocTokens()
end
alt meeting chain all failed AND meeting_notes non-empty
note over CLI: Fallback: return result with only meeting_notes tokens
else meeting chain all failed AND meeting_notes empty
note over CLI: Return error
end
Reviews (3): Last reviewed commit: "refactor(vc): remove ai_meeting_notes co..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@02fb2e23f98c975b51f95f26fd79ae87e6ff6f5f🧩 Skill updatenpx skills add larksuite/cli#feat/calendar-to-notes -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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/vc/vc_notes.go (1)
127-130:⚠️ Potential issue | 🟠 MajorDon't fail the
needNotespath before reading relation note tokens.Line 128 still requires
meeting_instance_idsto be non-empty beforemeeting_notes/ai_meeting_notesare parsed. That makes the new direct-relation fallback unreachable for events where the API returns note tokens but no meeting IDs, sofetchNoteByCalendarEventIDwill return an error instead of the tokens.Suggested fix
info, _ := infos[0].(map[string]any) - rawIDs, _ := info["meeting_instance_ids"].([]any) - if len(rawIDs) == 0 { - return nil, fmt.Errorf("no associated video meeting for this event") - } - - result := &eventRelationInfo{} + result := &eventRelationInfo{ + MeetingNotes: extractStringSlice(info, "meeting_notes"), + AIMeetingNotes: extractStringSlice(info, "ai_meeting_notes"), + } + rawIDs, _ := info["meeting_instance_ids"].([]any) + if len(rawIDs) == 0 && len(result.MeetingNotes) == 0 && len(result.AIMeetingNotes) == 0 { + return nil, fmt.Errorf("no associated video meeting for this event") + } for _, mid := range rawIDs { ... } - - result.MeetingNotes = extractStringSlice(info, "meeting_notes") - result.AIMeetingNotes = extractStringSlice(info, "ai_meeting_notes")Also applies to: 149-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/vc/vc_notes.go` around lines 127 - 130, The early return that errors when meeting_instance_ids is empty prevents the new direct-relation note-token fallback from ever being used; modify the logic in the code handling info (e.g., inside fetchNoteByCalendarEventID / the block referencing rawIDs, meeting_notes and ai_meeting_notes) so you first attempt to read/parse "meeting_notes" and "ai_meeting_notes" tokens and only if those are absent/empty then fall back to inspecting "meeting_instance_ids" and returning an error if both sources are empty. Apply the same reorder/fix to the second identical block around the 149-153 region so the direct-relation tokens are checked before failing on missing meeting IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/vc/vc_notes_test.go`:
- Around line 529-530: The new integration tests (e.g.,
TestNotes_CalendarPath_MeetingNotesDedup) call cmdutil.TestFactory without
isolating the CLI config, which can leak state; before creating the factory call
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) in
TestNotes_CalendarPath_MeetingNotesDedup and the other new tests that call
cmdutil.TestFactory (the other callers around the same block), or move that line
into a shared test helper so every TestFactory invocation in these tests runs
with an isolated config dir.
In `@shortcuts/vc/vc_notes.go`:
- Around line 183-188: The formatter that converts the JSON envelope to
text/table output still only renders note_doc_token, verbatim_doc_token, and
shared_doc_tokens and keys rows off meeting_id/minute_token, so add handling for
the new result keys "meeting_notes" and "ai_meeting_notes" (same place that
references note_doc_token, verbatim_doc_token, shared_doc_tokens and the row key
logic using meeting_id/minute_token) so relation-only successes include those
tokens in text and table modes; ensure the formatter emits them the same way as
the existing *_doc_token fields and include them when building rows for
relation-only results (also apply the same change where the code checks/returns
payloads around lines 208-210).
---
Outside diff comments:
In `@shortcuts/vc/vc_notes.go`:
- Around line 127-130: The early return that errors when meeting_instance_ids is
empty prevents the new direct-relation note-token fallback from ever being used;
modify the logic in the code handling info (e.g., inside
fetchNoteByCalendarEventID / the block referencing rawIDs, meeting_notes and
ai_meeting_notes) so you first attempt to read/parse "meeting_notes" and
"ai_meeting_notes" tokens and only if those are absent/empty then fall back to
inspecting "meeting_instance_ids" and returning an error if both sources are
empty. Apply the same reorder/fix to the second identical block around the
149-153 region so the direct-relation tokens are checked before failing on
missing meeting IDs.
🪄 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: 04491c61-dc1a-4e03-b2f9-d6979f3c4dbb
📒 Files selected for processing (4)
shortcuts/vc/vc_notes.goshortcuts/vc/vc_notes_test.goshortcuts/vc/vc_recording.goshortcuts/vc/vc_recording_test.go
Ren1104
force-pushed
the
feat/calendar-to-notes
branch
from
2cc33c8 to
02fb2e2
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/vc/vc_notes.go (1)
125-128:⚠️ Potential issue | 🟠 MajorAllow relation-note fallback when no
meeting_instance_idsare returned.At Line 126, the function errors before considering
meeting_notes. This prevents success for relation-only results even though note tokens may already be available.🔧 Suggested patch
- rawIDs, _ := info["meeting_instance_ids"].([]any) - if len(rawIDs) == 0 { - return nil, fmt.Errorf("no associated video meeting for this event") - } - - result := &eventRelationInfo{} + result := &eventRelationInfo{ + MeetingNotes: extractStringSlice(info, "meeting_notes"), + } + rawIDs, _ := info["meeting_instance_ids"].([]any) + if len(rawIDs) == 0 { + if needNotes && len(result.MeetingNotes) > 0 { + return result, nil + } + return nil, fmt.Errorf("no associated video meeting for this event") + } @@ - result.MeetingNotes = extractStringSlice(info, "meeting_notes") - return result, nilAlso applies to: 147-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/vc/vc_notes.go` around lines 125 - 128, The code currently returns an error when rawIDs (from info["meeting_instance_ids"]) is empty, preventing a fallback to relation-only notes; change the logic around the rawIDs check (the block with rawIDs, _ := info["meeting_instance_ids"]) to not immediately return an error but instead check for available meeting_notes (info["meeting_notes"]) and proceed with relation-note processing when notes exist, only returning an error if both meeting_instance_ids and meeting_notes are absent; apply the same fix to the similar block at lines handling the second rawIDs check (the repeated rawIDs/meeting_instance_ids block) so both places attempt meeting_notes fallback before erroring.
♻️ Duplicate comments (1)
shortcuts/vc/vc_notes_test.go (1)
515-517:⚠️ Potential issue | 🟠 MajorIsolate CLI config state in the new TestFactory-based tests.
These new tests create factories without setting an isolated config directory, which can make them order-dependent and leak machine-local state.
🔧 Suggested patch
func TestNotes_CalendarPath_MeetingNotesDedup(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) @@ func TestNotes_CalendarPath_FallbackWhenMeetingChainFails(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) @@ func TestNotes_CalendarPath_NeedNotes_RequestBody(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) warmTokenCache(t)As per coding guidelines,
**/*_test.go: Isolate config state in Go tests by usingt.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()).Also applies to: 552-554, 592-594
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/vc/vc_notes_test.go` around lines 515 - 517, The tests that call cmdutil.TestFactory (e.g., TestNotes_CalendarPath_MeetingNotesDedup) don't isolate CLI config state and may leak machine-local settings; add t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) at the start of each affected test (including TestNotes_CalendarPath_MeetingNotesDedup and the sibling tests that also call cmdutil.TestFactory) so each test uses its own temporary config directory before calling cmdutil.TestFactory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@shortcuts/vc/vc_notes.go`:
- Around line 125-128: The code currently returns an error when rawIDs (from
info["meeting_instance_ids"]) is empty, preventing a fallback to relation-only
notes; change the logic around the rawIDs check (the block with rawIDs, _ :=
info["meeting_instance_ids"]) to not immediately return an error but instead
check for available meeting_notes (info["meeting_notes"]) and proceed with
relation-note processing when notes exist, only returning an error if both
meeting_instance_ids and meeting_notes are absent; apply the same fix to the
similar block at lines handling the second rawIDs check (the repeated
rawIDs/meeting_instance_ids block) so both places attempt meeting_notes fallback
before erroring.
---
Duplicate comments:
In `@shortcuts/vc/vc_notes_test.go`:
- Around line 515-517: The tests that call cmdutil.TestFactory (e.g.,
TestNotes_CalendarPath_MeetingNotesDedup) don't isolate CLI config state and may
leak machine-local settings; add t.Setenv("LARKSUITE_CLI_CONFIG_DIR",
t.TempDir()) at the start of each affected test (including
TestNotes_CalendarPath_MeetingNotesDedup and the sibling tests that also call
cmdutil.TestFactory) so each test uses its own temporary config directory before
calling cmdutil.TestFactory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87917636-3c7b-4087-bf29-8286561699ab
📒 Files selected for processing (4)
shortcuts/vc/vc_notes.goshortcuts/vc/vc_notes_test.goskills/lark-vc/SKILL.mdskills/lark-vc/references/lark-vc-notes.md
✅ Files skipped from review due to trivial changes (2)
- skills/lark-vc/SKILL.md
- skills/lark-vc/references/lark-vc-notes.md
zhaoleibd
approved these changes
Apr 9, 2026
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
The
--calendar-event-idspath invc +notesnow extractsmeeting_notesandai_meeting_notesdoc tokens directly from themget_instance_relation_infoAPI response, in addition to the existing meeting_id → note detail chain. Tokens from both sources are deduplicated before output.Changes
resolveMeetingIDsFromCalendarEvent: returns*eventRelationInfo(was[]string), requestsneed_meeting_notes/need_ai_meeting_noteswhenneedNotes=truefetchNoteByCalendarEventID: merges doc tokens from both sources with deduplication; falls back to mget tokens when meeting chain failsvc_recording.go: adapted to new function signature (needNotes=false, no behavior change)extractStringSlice,deduplicateDocTokens,filterUnseen, calendar path integration (dedup + fallback), and request body verificationTest plan
extractStringSlice,toStringSlice,filterUnseen,deduplicateDocTokens)needNotes=truesends correct request body fieldsgo test ./shortcuts/vc/)Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
Documentation