Skip to content

feat(common): add FlagHints + did-you-mean unknown-flag error enhancemen#906

Open
xzcong0820 wants to merge 1 commit into
larksuite:mainfrom
xzcong0820:feat/8f59c0d
Open

feat(common): add FlagHints + did-you-mean unknown-flag error enhancemen#906
xzcong0820 wants to merge 1 commit into
larksuite:mainfrom
xzcong0820:feat/8f59c0d

Conversation

@xzcong0820
Copy link
Copy Markdown
Collaborator

@xzcong0820 xzcong0820 commented May 15, 2026

Generated by the harness-coding skill.

  • Branch: feat/8f59c0d
  • Target: main

Sprints

ID Title Status Commit
S1 Synthesize transport contract for larksuite/cli passed
S2 Implement Shortcut unknown-flag error enhancement (FlagHints + Did-You-Mean) passed 8061341

Source specs


This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.

Summary by CodeRabbit

  • New Features

    • Added “did you mean” suggestions for mistyped flag names using improved distance-based matching and dynamic thresholding.
    • Added per-shortcut flag alias hints so common aliases map to canonical flags, improving suggestions for mail shortcuts.
  • Bug Fixes

    • Suppressed automatic usage output on flag-parse errors for cleaner, focused error messages and hints.
  • Tests

    • Added comprehensive unit tests and a benchmark covering suggestion logic, thresholding, hinting, and error wrapping.

Review Change Stack

@github-actions github-actions Bot added domain/mail PR touches the mail domain size/L Large or sensitive change across domains or core paths labels May 15, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a flag-name suggestion system for unknown-flag errors (rune-aware Levenshtein + dynamic threshold), integrates it into Shortcut error handling via FlagHints and a custom SetFlagErrorFunc (SilenceUsage), and configures FlagHints for multiple mail shortcuts.

Changes

Flag suggestion system with mail shortcut hints

Layer / File(s) Summary
Core suggestion algorithms
shortcuts/common/flag_suggest.go, shortcuts/common/flag_suggest_test.go
Implements unknown-flag extraction, rune-aware Levenshtein edit distance, dynamic suggestion threshold, known-flag collection, candidate selection, tests (including Unicode cases) and a benchmark.
Error wrapping orchestration
shortcuts/common/flag_suggest.go, shortcuts/common/flag_suggest_test.go
wrapFlagError applies prioritized logic: per-shortcut FlagHints exact matches, edit-distance fallback, unknown-flag without hint, and non-unknown-flag validation errors. Tests cover hint formatting, prioritization, silenced usage behavior, and edge cases.
Shortcut framework integration
shortcuts/common/types.go, shortcuts/common/runner.go
Adds FlagHints map[string]string to Shortcut and installs a custom SetFlagErrorFunc with SilenceUsage: true for declarative commands.
Mail shortcut flag hints
shortcuts/mail/mail_draft_edit.go, shortcuts/mail/mail_message.go, shortcuts/mail/mail_send.go, shortcuts/mail/mail_thread.go, shortcuts/mail/mail_triage.go
Adds FlagHints mappings for mail shortcuts to map common alias flags to canonical flag names used by the shortcuts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • larksuite/cli#806: Overlapping work on Cobra “unknown flag” handling with “did you mean” suggestions for the mail command tree.

Suggested reviewers

  • chanthuang
  • liangshuo-1

Poem

🐰 I sniff the flags at break of day,
a typo hops and loses its way.
With whisker wisdom, soft and keen:
"Did you mean --that?" — a kindly glean.
Hop, fix, and on we go — clean!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete. It contains only auto-generated metadata and sprint tracking information with no summary, detailed changes list, test plan, or issue references as specified in the template. Add a Summary section describing the motivation, a Changes section listing the key modifications (FlagHints field, error wrapping logic, mail shortcut hints), a Test Plan documenting test coverage, and Related Issues section.
Docstring Coverage ⚠️ Warning Docstring coverage is 48.15% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title correctly identifies the main feature addition (FlagHints) and the key enhancement (did-you-mean unknown-flag error handling), matching the scope of changes across all modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@0dcbc8f58825283a89d76febd00a99b41ba66c97

🧩 Skill update

npx skills add xzcong0820/larksuite-cli#feat/8f59c0d -y -g

@xzcong0820 xzcong0820 force-pushed the feat/8f59c0d branch 3 times, most recently from 05e6c92 to 11e5684 Compare May 18, 2026 03:21
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
shortcuts/common/runner.go (1)

725-741: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Scope SilenceUsage to flag-parse errors only

Line 728 sets SilenceUsage: true at the command level, which suppresses usage output globally for all errors—including positional-arg rejection errors and RunE errors. This exceeds the stated intent ("prevent cobra from printing usage text on flag-parse errors"). Move SilenceUsage = true into the flag error handler to limit suppression to flag-parse errors only.

💡 Proposed fix
 	cmd := &cobra.Command{
 		Use:          shortcut.Command,
 		Short:        shortcut.Description,
 		Hidden:       shortcut.Hidden,
-		SilenceUsage: true, // prevent cobra from printing usage text on flag-parse errors
 		Args:         rejectPositionalArgs(),
 		RunE: func(cmd *cobra.Command, _ []string) error {
 			return runShortcut(cmd, f, &shortcut, botOnly)
 		},
 	}
@@
 	sc := shortcut // capture loop variable (required for go < 1.22; safe for go >= 1.22)
 	cmd.SetFlagErrorFunc(func(c *cobra.Command, err error) error {
+		c.SilenceUsage = true // only silence usage for flag-parse errors
 		return wrapFlagError(&sc, c, err)
 	})
🤖 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/common/runner.go` around lines 725 - 741, The command currently
sets SilenceUsage: true on the cobra.Command (in the command literal used by
runShortcut), which suppresses usage for all errors; instead leave SilenceUsage
false in the command and set c.SilenceUsage = true only inside the
SetFlagErrorFunc handler so only flag-parse errors are silenced. Concretely:
remove or unset SilenceUsage from the command creation, and in the anonymous
function passed to cmd.SetFlagErrorFunc (which currently calls
wrapFlagError(&sc, c, err)), set c.SilenceUsage = true before delegating to
wrapFlagError so RunE/positional-arg rejections still trigger usage output; keep
references to sc (captured shortcut), wrapFlagError,
registerShortcutFlagsWithContext, and runShortcut unchanged.
🤖 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.

Outside diff comments:
In `@shortcuts/common/runner.go`:
- Around line 725-741: The command currently sets SilenceUsage: true on the
cobra.Command (in the command literal used by runShortcut), which suppresses
usage for all errors; instead leave SilenceUsage false in the command and set
c.SilenceUsage = true only inside the SetFlagErrorFunc handler so only
flag-parse errors are silenced. Concretely: remove or unset SilenceUsage from
the command creation, and in the anonymous function passed to
cmd.SetFlagErrorFunc (which currently calls wrapFlagError(&sc, c, err)), set
c.SilenceUsage = true before delegating to wrapFlagError so RunE/positional-arg
rejections still trigger usage output; keep references to sc (captured
shortcut), wrapFlagError, registerShortcutFlagsWithContext, and runShortcut
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6e1968a3-7606-4271-9ab5-081079c601cf

📥 Commits

Reviewing files that changed from the base of the PR and between 05e6c92 and 11e5684.

📒 Files selected for processing (9)
  • shortcuts/common/flag_suggest.go
  • shortcuts/common/flag_suggest_test.go
  • shortcuts/common/runner.go
  • shortcuts/common/types.go
  • shortcuts/mail/mail_draft_edit.go
  • shortcuts/mail/mail_message.go
  • shortcuts/mail/mail_send.go
  • shortcuts/mail/mail_thread.go
  • shortcuts/mail/mail_triage.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • shortcuts/mail/mail_message.go
  • shortcuts/mail/mail_send.go
  • shortcuts/mail/mail_thread.go
  • shortcuts/common/flag_suggest.go

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.39%. Comparing base (f03138b) to head (0dcbc8f).
⚠️ Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/common/runner.go 33.33% 3 Missing and 1 partial ⚠️
shortcuts/mail/flag_suggest.go 91.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #906      +/-   ##
==========================================
+ Coverage   65.95%   67.39%   +1.44%     
==========================================
  Files         523      572      +49     
  Lines       49590    53683    +4093     
==========================================
+ Hits        32707    36182    +3475     
- Misses      14090    14491     +401     
- Partials     2793     3010     +217     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xzcong0820 xzcong0820 force-pushed the feat/8f59c0d branch 3 times, most recently from 5308ea8 to 23f795d Compare May 19, 2026 08:49
Adds a per-shortcut FlagHints map (common/types.go) that lets mail
shortcuts declare known flag aliases (e.g. "recipient" → "to").

At mount time, runner.go serialises FlagHints into cmd.Annotations so
the data is accessible at error-handling time without creating a circular
import between common and mail.

mail/flag_suggest.go's flagSuggestErrorFunc gains a Priority-0 path that
reads the hint map from cmd.Annotations and inserts the exact-match
target as candidates[0] with reason="hint" and distance=0, before the
existing prefix + Levenshtein fallback. The hint target is deduplicated
from the suggest() results so it never appears twice.

FlagHints is configured on five mail shortcuts:
- mail_draft_edit, mail_message, mail_send, mail_thread, mail_triage

Output format (candidates[] array, error.type="unknown_flag") is
unchanged from PR larksuite#806.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/mail PR touches the mail domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants