feat: replace words for transcript#1372
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds three minutes CLI shortcuts— ChangesMinutes Shortcuts: Summary, Todo, Word Replace
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
github-actions
Bot
added
domain/task
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1372 +/- ##
==========================================
+ Coverage 71.90% 71.95% +0.05%
==========================================
Files 691 699 +8
Lines 65629 66164 +535
==========================================
+ Hits 47191 47611 +420
- Misses 14791 14864 +73
- Partials 3647 3689 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@4f22f4444c0c7e5ef39078a5bb83932fb936562e🧩 Skill updatenpx skills add larksuite/cli#feat/transcript_word_replace0601 -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: 6
🧹 Nitpick comments (1)
shortcuts/minutes/minutes_summary_todo_test.go (1)
250-252: ⚖️ Poor tradeoffReconsider exposing
todo_idconstraint in output validation.Line 250-252 asserts that the
todo_id"must never be surfaced to the user in the command output". While hiding internal IDs is a reasonable UX decision, enforcing it via test assertions that scanstdoutfor the literal value"88"is fragile (e.g., it would break if a different field happened to contain "88", or if output formatting changes).Consider whether this constraint belongs in the test suite or in code review/UX guidelines. If it must be tested, document the rationale clearly in a comment.
🤖 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/minutes/minutes_summary_todo_test.go` around lines 250 - 252, The test currently asserts that stdout must not contain the literal "88" (using stdout.String() and t.Errorf), which is fragile; update the test to either remove this brittle literal-check or make it robust: parse the command output (e.g., as JSON or structured fields) and assert that there is no "todo_id" key, or change the assertion to check for the presence/absence of a labeled field (like "todo_id") rather than the numeric value "88"; if you keep the constraint, add a clear inline comment explaining why hiding the ID is required and reference stdout/String() and the failing t.Errorf call so reviewers know this is an intentional UX test.
🤖 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/minutes/minutes_summary_todo_test.go`:
- Around line 103-120: Update the test TestMinutesTodo_RequiresIsDone to assert
the typed validation error metadata instead of substring matching: when
mountAndRun (used in MinutesTodo tests) returns an error, assert the error is a
validation error created via errs.NewValidationError by using errs.ProblemOf to
check category/subtype/parameter (e.g., that ProblemOf(err) reports validation
and the parameter "is-done"), and also assert the original cause is preserved
via errs.Cause or similar helper; apply the same replacement pattern to
TestMinutesTodo_RequiresOperation and other validation error tests that
currently use strings.Contains on err.Error().
In `@shortcuts/minutes/minutes_summary.go`:
- Around line 36-49: The Validate function currently returns legacy
output.ErrValidation in three places (when minuteToken == "" , when
validate.ResourceName fails, and when summary == "" ); replace these with typed
validation errors using errs.NewValidationError(errs.SubtypeInvalidArgument,
"<message>") and attach the offending flag via .WithParam("--minute-token") or
.WithParam("--summary"); for the validate.ResourceName error wrap the returned
err into errs.NewValidationError(errs.SubtypeInvalidArgument,
err.Error()).WithParam("--minute-token") so command-facing failures are typed
correctly and include param metadata.
In `@shortcuts/minutes/minutes_todo.go`:
- Around line 65-77: In the Validate function replace legacy
output.ErrValidation calls with typed errs validation errors: when minuteToken
== "" return errs.NewValidationError(errs.SubtypeInvalidArgument, "missing
required flag").WithParam("--minute-token"); when
validate.ResourceName(minuteToken, "--minute-token") returns err wrap it as
errs.NewValidationError(errs.SubtypeInvalidArgument, "%s",
err).WithParam("--minute-token"); keep other logic (resolveMinuteTodoOps)
unchanged and ensure the returned error types are the new errs.* typed
validation errors so command-facing failures use the proper subtype and param
metadata.
- Around line 180-229: In resolveMinuteTodoSingle replace all uses of
output.ErrValidation with errs.NewValidationError(errs.SubtypeInvalidArgument,
"<message>").WithParam("<flag>") and return that error; specifically: the
missing operation case should use WithParam("--operation"); the "add" case
missing args should use WithParam("--todo") (and/or "--is-done" — prefer the
primary missing flag "--todo"); the "add" todo-id presence error should use
WithParam("--todo-id"); the "update" missing fields error should use
WithParam("--todo-id"); the "delete" missing todo-id error should use
WithParam("--todo-id"); the "delete" unexpected flags error should use
WithParam("--todo") (or "--is-done" as appropriate); and the default
unknown-operation error should use WithParam("--operation"); keep the rest of
resolveMinuteTodoSingle logic (minuteTodoSpec, buildMinuteTodoItem,
minuteTodoOp) unchanged. Replace the final build error return to wrap/convert
into a validation error similarly if it represents bad input.
- Around line 128-144: The file uses the legacy output.ErrValidation helper;
replace those calls with typed errs.NewValidationError calls: in
resolveMinuteTodoOps return errs.NewValidationError(errs.SubtypeInvalidArgument,
"use either --todos for batch or single-item flags (--operation, --todo,
--is-done, --todo-id), not both").WithParam("--todos") instead of
output.ErrValidation, and similarly replace the other output.ErrValidation
usages in this file (including those in
resolveMinuteTodoBatch/resolveMinuteTodoSingle flows) with
errs.NewValidationError(errs.SubtypeInvalidArgument, "<same validation
message>").WithParam("<relevant-flag>") using the specific flag name that caused
the validation (e.g., "--todos", "--operation", "--todo", "--is-done", or
"--todo-id") so each validation error is typed and carries the correct
parameter.
In `@shortcuts/minutes/minutes_word_replace.go`:
- Around line 48-60: The Validate function currently returns legacy
output.ErrValidation values; replace those with typed validation errors using
errs.NewValidationError(errs.SubtypeInvalidArgument,
"<message>").WithParam("--minute-token") for the minute-token checks and
errs.NewValidationError(errs.SubtypeInvalidArgument,
"<message>").WithParam("--replace-words") for the replace-words parse error;
update the three error returns in the Validate closure (the empty minuteToken
check, the validate.ResourceName(minuteToken, "--minute-token") error branch,
and the parseReplaceWords error branch) to construct and return the
corresponding errs.NewValidationError(...) instances preserving the original
messages and include the exact flag name via WithParam.
---
Nitpick comments:
In `@shortcuts/minutes/minutes_summary_todo_test.go`:
- Around line 250-252: The test currently asserts that stdout must not contain
the literal "88" (using stdout.String() and t.Errorf), which is fragile; update
the test to either remove this brittle literal-check or make it robust: parse
the command output (e.g., as JSON or structured fields) and assert that there is
no "todo_id" key, or change the assertion to check for the presence/absence of a
labeled field (like "todo_id") rather than the numeric value "88"; if you keep
the constraint, add a clear inline comment explaining why hiding the ID is
required and reference stdout/String() and the failing t.Errorf call so
reviewers know this is an intentional UX test.
🪄 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: 30f73c5f-9192-45aa-8139-65c585497c71
📒 Files selected for processing (12)
shortcuts/minutes/minutes_summary.goshortcuts/minutes/minutes_summary_todo_test.goshortcuts/minutes/minutes_todo.goshortcuts/minutes/minutes_word_replace.goshortcuts/minutes/minutes_word_replace_test.goshortcuts/minutes/shortcuts.goskills/lark-minutes/SKILL.mdskills/lark-minutes/references/lark-minutes-summary.mdskills/lark-minutes/references/lark-minutes-todo.mdskills/lark-task/SKILL.mdskills/lark-vc/references/lark-vc-notes.mdtests/cli_e2e/minutes/minutes_word_replace_test.go
calendar-assistant
previously approved these changes
Jun 10, 2026
zhangjun-bytedance
force-pushed
the
feat/transcript_word_replace0601
branch
from
June 10, 2026 06:08
89aaeb7 to
071cd37
Compare
zhangjun-bytedance
force-pushed
the
feat/transcript_word_replace0601
branch
from
June 10, 2026 06:16
071cd37 to
154f2fb
Compare
zhangjun-bytedance
force-pushed
the
feat/transcript_word_replace0601
branch
from
June 10, 2026 06:28
154f2fb to
4f22f44
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/minutes/minutes_word_replace.go (1)
116-126: 💤 Low valueNote: asymmetric trimming for source vs target words.
source_wordis trimmed (line 116) buttarget_wordis passed through without trimming (line 126). This asymmetry is intentional per the AI summary but could surprise users who accidentally include trailing whitespace intarget_word. The current design prioritizes flexibility (allowing intentional whitespace in replacements) over defensive sanitization.If you want stricter input handling, trim
target_wordas well:Optional: trim target_word for consistency
seen[sourceWord] = struct{}{} replaceWords = append(replaceWords, map[string]string{ "source_word": sourceWord, - "target_word": item.TargetWord, + "target_word": strings.TrimSpace(item.TargetWord), })🤖 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/minutes/minutes_word_replace.go` around lines 116 - 126, The code trims source_word but leaves target_word untrimmed, causing inconsistent handling; update the construction of replaceWords so that item.TargetWord is trimmed (e.g., use strings.TrimSpace(item.TargetWord) when assigning "target_word") and adjust any duplicate-detection or validation logic if it relies on trimmed values (variables referenced: sourceWord, item.TargetWord, seen, replaceWords, the map keys "source_word" and "target_word").
🤖 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/minutes/shortcuts.go`:
- Around line 15-16: Add end-to-end tests that exercise the MinutesSummary and
MinutesTodo shortcuts: create dry-run E2E cases in the tests/cli_e2e suite that
invoke the CLI with the "+summary" and "+todo" shortcuts and assert expected
outputs; if these shortcuts are newly added behavior, also add live E2E runs.
Locate the unit test references in
shortcuts/minutes/minutes_summary_todo_test.go and mirror those scenarios in new
E2E test files under tests/cli_e2e (calling the CLI binary and checking
stdout/stderr), ensuring the tests invoke the shortcuts by name (MinutesSummary,
MinutesTodo) and validate their outputs. Ensure tests are labeled as dry-run
(and add live variants only if appropriate) and follow existing E2E test
patterns for setup/teardown and assertions.
---
Nitpick comments:
In `@shortcuts/minutes/minutes_word_replace.go`:
- Around line 116-126: The code trims source_word but leaves target_word
untrimmed, causing inconsistent handling; update the construction of
replaceWords so that item.TargetWord is trimmed (e.g., use
strings.TrimSpace(item.TargetWord) when assigning "target_word") and adjust any
duplicate-detection or validation logic if it relies on trimmed values
(variables referenced: sourceWord, item.TargetWord, seen, replaceWords, the map
keys "source_word" and "target_word").
🪄 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: 842a6ed5-8170-4483-b955-087399ed6143
📒 Files selected for processing (14)
shortcuts/minutes/minutes_speaker_replace_test.goshortcuts/minutes/minutes_summary.goshortcuts/minutes/minutes_summary_todo_test.goshortcuts/minutes/minutes_todo.goshortcuts/minutes/minutes_update_test.goshortcuts/minutes/minutes_word_replace.goshortcuts/minutes/minutes_word_replace_test.goshortcuts/minutes/shortcuts.goskills/lark-minutes/SKILL.mdskills/lark-minutes/references/lark-minutes-summary.mdskills/lark-minutes/references/lark-minutes-todo.mdskills/lark-task/SKILL.mdskills/lark-vc/references/lark-vc-notes.mdtests/cli_e2e/minutes/minutes_word_replace_test.go
✅ Files skipped from review due to trivial changes (3)
- skills/lark-vc/references/lark-vc-notes.md
- shortcuts/minutes/minutes_update_test.go
- shortcuts/minutes/minutes_speaker_replace_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/cli_e2e/minutes/minutes_word_replace_test.go
- skills/lark-task/SKILL.md
- shortcuts/minutes/minutes_todo.go
- skills/lark-minutes/SKILL.md
- shortcuts/minutes/minutes_summary.go
calendar-assistant
approved these changes
Jun 10, 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
Changes
Test Plan
lark-cli <domain> <command>flow works as expectedRelated Issues
Summary by CodeRabbit
New Features
Bug Fixes / Error Handling
Tests
Documentation