feat(cmdutil): add did-you-mean suggestions for unknown flags#768
feat(cmdutil): add did-you-mean suggestions for unknown flags#768xzcong0820 wants to merge 1 commit intolarksuite:mainfrom
Conversation
|
ℹ️ 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)
📝 WalkthroughWalkthroughThis PR adds a structured CLI flag suggestion system with "did you mean" hints for unknown flags. It introduces ChangesCLI Flag Suggestion with "Did You Mean" Hints
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
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 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
the
size/L
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@7f84817aba8e858cbdcad46fb7071199ebb527e2🧩 Skill updatenpx skills add xzcong0820/larksuite-cli#feature/cli-flag-did-you-mean -y -g |
xzcong0820
force-pushed
the
feature/cli-flag-did-you-mean
branch
from
770da89 to
89bd750
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/cmdutil/flag_suggest_test.go (1)
20-31: ⚡ Quick winUse the repo-standard test factory helper for unit tests.
Please replace the ad-hoc command builder with the standardized factory path to keep test setup consistent across the suite.
As per coding guidelines: "
**/*_test.go: Usecmdutil.TestFactory(t, config)for test factories in unit tests".🤖 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 `@internal/cmdutil/flag_suggest_test.go` around lines 20 - 31, Replace the ad-hoc builder newTriageLikeCmd used in the test with the repository-standard test factory by calling cmdutil.TestFactory(t, config) (i.e., TestFactory(t, config)) and configuring the returned factory/command to expose the same structure and flags: ensure the root persistent "format" flag exists and that a child command "+triage" is added with flags "max", "filter", "page-size" and the hidden "internal" flag (MarkHidden("internal")). Remove newTriageLikeCmd and adapt the test to use the factory-provided command/runner so the test setup follows the repo convention.
🤖 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 `@internal/cmdutil/flag_suggest_test.go`:
- Around line 20-31: Replace the ad-hoc builder newTriageLikeCmd used in the
test with the repository-standard test factory by calling cmdutil.TestFactory(t,
config) (i.e., TestFactory(t, config)) and configuring the returned
factory/command to expose the same structure and flags: ensure the root
persistent "format" flag exists and that a child command "+triage" is added with
flags "max", "filter", "page-size" and the hidden "internal" flag
(MarkHidden("internal")). Remove newTriageLikeCmd and adapt the test to use the
factory-provided command/runner so the test setup follows the repo convention.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 720942a2-99be-4c52-8f64-659976d06478
📒 Files selected for processing (4)
cmd/build.gointernal/cmdutil/flag_suggest.gointernal/cmdutil/flag_suggest_test.gotests/cli_e2e/flag_suggest_regression_test.go
✅ Files skipped from review due to trivial changes (1)
- tests/cli_e2e/flag_suggest_regression_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/build.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #768 +/- ##
==========================================
+ Coverage 65.21% 65.45% +0.24%
==========================================
Files 504 509 +5
Lines 46673 46888 +215
==========================================
+ Hits 30437 30692 +255
+ Misses 13597 13551 -46
- Partials 2639 2645 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Wire UnknownFlagHandler onto the root command via SetFlagErrorFunc.
cobra walks up to the root for FlagErrorFunc, so a single install on
rootCmd applies CLI-wide.
The handler covers two layers:
- synonym map for cross-CLI conventions agents reach for first
(--limit -> --max, --folder -> --filter with extra hint, etc.).
Only fires when the canonical target flag is registered on the
invoked command.
- Levenshtein fallback with threshold max(2, len/3); hidden flags
excluded; ties broken alphabetically.
Short flags get a plain "unknown flag" message -- pflag's short-flag
namespace is separate from long flags and 1-char Levenshtein matches
would be near-meaningless.
Other flag-error types (required missing, type mismatch, etc.) pass
through unchanged so cobra's default semantics apply. Output is the
structured ExitError envelope from output.ErrValidation, satisfying
the AGENTS.md contract for AI agents to pattern-match `error.hint`.
xzcong0820
force-pushed
the
feature/cli-flag-did-you-mean
branch
from
89bd750 to
7f84817
Compare
|
Thanks for the thoughtful work here — the architecture choice (single-point install via cobra's FlagErrorFunc walk-up) and the test Heads-up: we're currently working on a broader refactor of the global hint / error-message system across the CLI, which overlaps |
2 participants
Summary
Wire
cmdutil.UnknownFlagHandleronto the root command viaSetFlagErrorFuncso that unknown CLI flags produce the structuredExitErrorenvelope with an optionalhintfield (e.g."did you mean --max?"). AI agents that consume lark-cli output viaAGENTS.md pattern-match
error.hintto recover from typos; today anunknown flag yields a generic message and the agent has no signal for
what to retry with. cobra walks up to the root for
FlagErrorFunc, soa single install on
rootCmdapplies CLI-wide.Changes
internal/cmdutil/flag_suggest.gowithUnknownFlagHandlerandSuggestFlag. Two-layer matching: a curated synonym map (other-CLIconventions agents commonly reach for, e.g.
--limit -> --max,--folder -> --filter) followed by a Levenshtein fallback withthreshold
max(2, len/3). Hidden flags excluded; ties brokenalphabetically.
on the invoked command, so we don't mislead users on commands that
don't define it.
namespace is separate from long flags and 1-char Levenshtein matches
would be near-meaningless.
through unchanged so cobra's default semantics apply.
cmd/build.goviarootCmd.SetFlagErrorFunc.Behavior reference
--limit 5did you mean --max?--folder INBOXdid you mean --filter? pass folder via --filter '{"folder":"..."}'--maxx 5did you mean --max?--halpdid you mean --help?-Z--totally-bogus-xyzTest Plan
go test ./internal/cmdutil/...coversflag_suggest_test.go(14 funcs incl. synonym hit/fallthrough,edit-distance, hidden-flag exclusion, tiebreaker, nil-defense,
pflag message parsing, handler pass-through).
go test ./tests/cli_e2e/...runsflag_suggest_regression_test.go(5 hit cases + pass-through).lark-cli mail +triage --limit 5 --dry-runand equivalents across
calendar,drive,im,okrsubcommands all return the structured envelope with the expected
hint.Summary by CodeRabbit