fix: add vc-domain-boundaries and enrich vc +notes#1172
Conversation
hugang-lark
added
documentation
|
|
github-actions
Bot
added
the
size/L
📝 WalkthroughWalkthroughThis PR extends the ChangesVC Meeting Note Artifact Retrieval
Search Shortcut Filter Description
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 docstrings
🧪 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.
🧹 Nitpick comments (1)
skills/lark-vc/references/vc-domain-boundaries.md (1)
58-68: ⚡ Quick winAdd language identifiers to code blocks for better rendering.
The code blocks at lines 58-68 (transcript format examples) and lines 142-155 (domain relationship diagram) are missing language identifiers, which can cause inconsistent rendering across markdown processors.
📝 Proposed fix
-``` +```text 发言人名称 相对时间戳 <发言内容>Apply the same fix to the example at lines 65-68 and the diagram at lines 142-155: ```diff -``` +```text Calendar (日程) ──── 发起预约 ────► VC (会议) ...</details> Also applies to: 142-155 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@skills/lark-vc/references/vc-domain-boundaries.mdaround lines 58 - 68, The
shown Markdown code blocks for the transcript example ("发言人名称 相对时间戳\n<发言内容>" /
example "张三 00:00:00.195\n我们接下来讨论一下项目进度。") and the domain relationship diagram
("Calendar (日程) ──── 发起预约 ────► VC (会议) ...") lack language identifiers; update
their fences fromtotext for both the transcript block and the diagram
block so they render consistently across processors (locate the blocks
containing the exact strings "发言人名称 相对时间戳" and "Calendar (日程) ──── 发起预约 ────► VC
(会议)" and replace the opening triple backticks with ```text).</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@skills/lark-vc/references/vc-domain-boundaries.md:
- Around line 58-68: The shown Markdown code blocks for the transcript example
("发言人名称 相对时间戳\n<发言内容>" / example "张三 00:00:00.195\n我们接下来讨论一下项目进度。") and the
domain relationship diagram ("Calendar (日程) ──── 发起预约 ────► VC (会议) ...") lack
language identifiers; update their fences fromtotext for both the
transcript block and the diagram block so they render consistently across
processors (locate the blocks containing the exact strings "发言人名称 相对时间戳" and
"Calendar (日程) ──── 发起预约 ────► VC (会议)" and replace the opening triple backticks
with ```text).</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `e81d6c81-595f-4e99-9a29-77935e08a415` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e18ea9a2e824472c41c279ee80933a5d559a3d86 and a1d446d4df26e534f59e35cd85fd6e2a8596ba46. </details> <details> <summary>📒 Files selected for processing (8)</summary> * `shortcuts/vc/vc_notes.go` * `shortcuts/vc/vc_search.go` * `skills/lark-minutes/SKILL.md` * `skills/lark-vc/SKILL.md` * `skills/lark-vc/references/lark-vc-notes.md` * `skills/lark-vc/references/lark-vc-search.md` * `skills/lark-vc/references/vc-domain-boundaries.md` * `skills/lark-workflow-meeting-summary/SKILL.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@6dc95c7592b35b8904ecf689473bc4d0d555052f🧩 Skill updatenpx skills add larksuite/cli#feat/vc_event -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1172 +/- ##
==========================================
+ Coverage 68.92% 68.95% +0.03%
==========================================
Files 628 629 +1
Lines 58744 58832 +88
==========================================
+ Hits 40487 40567 +80
- Misses 14951 14960 +9
+ Partials 3306 3305 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hugang-lark
changed the title
hugang-lark
force-pushed
the
feat/vc_event
branch
from
a1d446d to
6dc95c7
Compare
hugang-lark
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
skills/lark-vc/references/vc-domain-boundaries.md (2)
58-68: ⚡ Quick winAdd language identifier to fenced code blocks.
The code blocks showing transcript format examples should specify a language identifier for proper Markdown rendering.
📝 Proposed fix to add language identifiers
-``` +```text 发言人名称 相对时间戳 <发言内容>示例：
-
+text
张三 00:00:00.195
我们接下来讨论一下项目进度。🤖 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 `@skills/lark-vc/references/vc-domain-boundaries.md` around lines 58 - 68, The fenced code blocks showing the transcript format (the block starting with "发言人名称 相对时间戳" and the example block starting with "张三 00:00:00.195") need a language identifier for proper Markdown rendering; update both triple-backtick fences to use "text" (i.e., change ``` to ```text) so the blocks explicitly declare the language and render correctly.
142-155: ⚡ Quick winAdd language identifier to ASCII diagram code block.
The code block containing the ASCII diagram should specify a language identifier for proper Markdown rendering.
📝 Proposed fix to add language identifier
-``` +```text Calendar (日程) ──── 发起预约 ────► VC (会议) │ ┌──────────────────┤🤖 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 `@skills/lark-vc/references/vc-domain-boundaries.md` around lines 142 - 155, The ASCII diagram code block in vc-domain-boundaries.md lacks a language identifier; update the fenced code block that starts with the diagram (the triple-backtick block containing "Calendar (日程) ──── 发起预约") to include a language tag such as "text" (i.e., change ``` to ```text) so Markdown renderers treat it as preformatted text and preserve the diagram formatting.
🤖 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_notes.go`:
- Around line 778-787: The success counting incorrectly treats echoed
minute_token entries as successes because hasNotesPayload sees a non-empty
"minute_token"; modify the successCount logic in Execute to be mode-aware: when
running the --minute-tokens path (the mode enforced by Execute/ExactlyOne),
count a result as successful only if the result map from fetchNoteByMinuteToken
contains no non-nil "error" (or error == nil), otherwise for the other paths
keep the existing hasNotesPayload check; update the loop over results (where
successCount is computed) to branch on the mode and check
r.(map[string]any)["error"] for minute-token mode so the all-failed exit and
summary match the per-row FAIL statuses, and re-run hasNotesPayload/minute-token
tests.
---
Nitpick comments:
In `@skills/lark-vc/references/vc-domain-boundaries.md`:
- Around line 58-68: The fenced code blocks showing the transcript format (the
block starting with "发言人名称 相对时间戳" and the example block starting with "张三
00:00:00.195") need a language identifier for proper Markdown rendering; update
both triple-backtick fences to use "text" (i.e., change ``` to ```text) so the
blocks explicitly declare the language and render correctly.
- Around line 142-155: The ASCII diagram code block in vc-domain-boundaries.md
lacks a language identifier; update the fenced code block that starts with the
diagram (the triple-backtick block containing "Calendar (日程) ──── 发起预约") to
include a language tag such as "text" (i.e., change ``` to ```text) so Markdown
renderers treat it as preformatted text and preserve the diagram formatting.
🪄 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: e8ce7a08-f8ac-41fd-af75-f6c29bdf9e58
📒 Files selected for processing (8)
shortcuts/vc/vc_notes.goshortcuts/vc/vc_notes_test.goshortcuts/vc/vc_search.goskills/lark-minutes/SKILL.mdskills/lark-vc/SKILL.mdskills/lark-vc/references/lark-vc-notes.mdskills/lark-vc/references/vc-domain-boundaries.mdskills/lark-workflow-meeting-summary/SKILL.md
✅ Files skipped from review due to trivial changes (3)
- shortcuts/vc/vc_search.go
- skills/lark-vc/SKILL.md
- skills/lark-vc/references/lark-vc-notes.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/lark-minutes/SKILL.md
- skills/lark-workflow-meeting-summary/SKILL.md
| // count results: a result counts as "successful" when it carries any | ||
| // note/minute payload, even if the merged `error` field surfaces a | ||
| // partial failure (e.g. note ok but minute_token lookup failed). | ||
| successCount := 0 | ||
| for _, r := range results { | ||
| m, _ := r.(map[string]any) | ||
| if m["error"] == nil { | ||
| if hasNotesPayload(m) { | ||
| successCount++ | ||
| } | ||
| } |
There was a problem hiding this comment.
Minute-tokens failures are counted as successes, suppressing the "all failed" exit error.
hasNotesPayload treats a non-empty minute_token as a usable payload, but on the --minute-tokens path fetchNoteByMinuteToken echoes the input token into the result even on failure (Line 405 {"minute_token": minuteToken, "error": ...} and Line 415). So every minute-token result — including permission-denied / "minutes not found" — has a non-empty minute_token and is counted as a success.
Consequences for the --minute-tokens mode:
successCountis alwayslen(results), so the "all failed" branch at Line 791 never fires and the command exits0even when every lookup failed.- The summary line (Line 788) and the table status (Line 811, which marks these
FAIL) disagree.
This is a regression from the previous error == nil success check. The echoed minute_token can't be distinguished from a discovered minute_token (the meeting-ids partial-success case at Line 361) by the result map alone, so the fix needs to be mode-aware. Execute already knows the mode (enforced by ExactlyOne):
🐛 Proposed mode-aware success counting
successCount := 0
+ // In --minute-tokens mode the result is keyed by the echoed minute_token,
+ // so its presence is not evidence of a payload; the lookup succeeded only
+ // when no error was recorded.
+ minuteMode := runtime.Str("minute-tokens") != ""
for _, r := range results {
m, _ := r.(map[string]any)
- if hasNotesPayload(m) {
+ if minuteMode {
+ if errMsg, _ := m["error"].(string); errMsg == "" {
+ successCount++
+ }
+ } else if hasNotesPayload(m) {
successCount++
}
}Please re-validate the hasNotesPayload / minute-token integration tests against this change.
🤖 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_notes.go` around lines 778 - 787, The success counting
incorrectly treats echoed minute_token entries as successes because
hasNotesPayload sees a non-empty "minute_token"; modify the successCount logic
in Execute to be mode-aware: when running the --minute-tokens path (the mode
enforced by Execute/ExactlyOne), count a result as successful only if the result
map from fetchNoteByMinuteToken contains no non-nil "error" (or error == nil),
otherwise for the other paths keep the existing hasNotesPayload check; update
the loop over results (where successCount is computed) to branch on the mode and
check r.(map[string]any)["error"] for minute-token mode so the all-failed exit
and summary match the per-row FAIL statuses, and re-run
hasNotesPayload/minute-token tests.
2 participants
Summary
Add a Calendar/VC/Doc cross-domain boundaries document and consequence-driven CRITICAL references across the
lark-vc/lark-minutes/lark-workflow-meeting-summaryskills, and harden thevc +notesshortcut so meeting-id queries also surface theminute_tokenand translate common API error codes into actionable hints. This prevents AI agents from skipping domain knowledge or making wrong artifact selections, and gives callers a single, structured response that covers both the AI-summary and recording pipelines in one shot.Changes
skills/lark-vc/references/vc-domain-boundaries.md: Calendar/VC/Doc relationships, the two independent meeting artifact pipelines, artifact selection rules, transcript format, and the 4-step meeting summary workflow.lark-vc/SKILL.md,lark-minutes/SKILL.md,lark-workflow-meeting-summary/SKILL.md,lark-vc-notes.md, andlark-vc-search.mdso key decision points point to the boundaries doc.[2091005] permission denyhandling tip forminutes.getinlark-minutes/SKILL.md.vc +notes(shortcuts/vc/vc_notes.go): reverse-lookupminute_tokenvia the recording API on the--meeting-ids/--calendar-event-idspaths, merge note-side and minute-side failures into a single;-joinederrorfield, and translate notes 121005 / recording 121004·121005·124002 / minutes 2091005 into friendly hints; update the docs and dry-run output accordingly.Test Plan
make unit-testpasseslark-cli vc +search,lark-cli vc +notes, andlark-cli minutes minutes getbehave as expected (including the newminute_tokenfield and the friendly error hints)Related Issues