Skip to content

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

Open
xzcong0820 wants to merge 2 commits into
larksuite:mainfrom
xzcong0820:feat/8f59c0d
Open

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

Conversation

@xzcong0820
Copy link
Copy Markdown
Collaborator

@xzcong0820 xzcong0820 commented May 15, 2026 •
edited by coderabbitai Bot
Loading

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 misspelled flag names.
    • Added per-shortcut flag alias hints so common aliases map to canonical flags (improves flag recognition in mail shortcuts).

  • Bug Fixes

    • Suppressed automatic usage help on flag-parse errors for cleaner, targeted error messages.

  • Tests

    • Added comprehensive unit tests and a benchmark for flag-suggestion and error-wrapping behavior.

Review Change Stack

@xzcong0820
feat(common): add FlagHints + did-you-mean unknown-flag error enhance…
8061341 
…ment

Add framework-level unknown-flag error interception in shortcuts/common/.
When a user or AI agent passes an unrecognized flag, cobra's FlagErrorFunc
now returns structured JSON (via output.ExitError) instead of plain-text,
with an optional "did you mean: --X?" hint.

Changes:
- types.go: add FlagHints map[string]string field to Shortcut struct
- flag_suggest.go: implement extractUnknownFlagName, editDistance, min3,
  flagSuggestThreshold, collectKnownFlagNamesFromCmd, didYouMeanFlag,
  wrapFlagError with four-priority logic (FlagHints > edit-distance >
  no-match > non-unknown-flag)
- runner.go: add SilenceUsage: true to cobra.Command literal and register
  SetFlagErrorFunc before parent.AddCommand in mountDeclarative
- mail shortcuts: populate FlagHints for +draft-edit, +message, +thread,
  +triage, +send with domain-specific misuse mappings

sprint: S2
@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 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a37a8119-131c-4df3-8a22-f224bed14c4e

📥 Commits

Reviewing files that changed from the base of the PR and between e50b162 and b06ba16.

📒 Files selected for processing (1)
  • shortcuts/common/flag_suggest.go
📝 Walkthrough

Walkthrough

Added a flag suggestion system that recommends corrections for unknown flag names using rune-aware Levenshtein distance and per-shortcut FlagHints; integrated into the shortcut framework and configured with domain-specific mail shortcut hints.

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 error parsing, rune-aware Levenshtein edit distance, dynamic suggestion threshold based on flag name length, and candidate selection from registered flags. Tests cover correctness across Unicode strings, threshold behavior, and benchmark.
Error wrapping orchestration
shortcuts/common/flag_suggest.go, shortcuts/common/flag_suggest_test.go		 Central wrapFlagError function applies prioritized logic: per-shortcut FlagHints exact matches, edit-distance fallback, unknown-flag without hint, and non-unknown-flag validation errors. Tests validate hint generation, formatting consistency, SilenceUsage behavior, and edge cases.
Shortcut framework integration
shortcuts/common/types.go, shortcuts/common/runner.go		 Adds FlagHints map[string]string field to Shortcut struct and installs custom SetFlagErrorFunc handler in declarative shortcuts with SilenceUsage: true to suppress usage text on flag errors.
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		 Configures FlagHints for mail shortcuts: +draft-edit, +message, +send, +thread, and +triage to map common aliases to canonical flag names.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • larksuite/cli#806: Overlapping mail-specific flag-suggest logic that this PR centralizes into the common shortcut framework.

Suggested labels

domain/mail, size/L

Suggested reviewers

  • chanthuang
  • liangshuo-1

Poem

🐰 I hop through flags both near and far,
sniffing typos like a tiny star.
A whisper: "Did you mean this?" I plea,
and the CLI replies with clarity.
Hooray — fewer typos for you and me!

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely off-topic and incomplete. It omits required template sections (Summary, Changes, Test Plan, Related Issues) and instead provides automation metadata (sprints, branch info) that doesn't explain the actual changes to reviewers. Replace with the standard template: add a clear Summary (1-3 sentences explaining motivation), list main Changes, describe Test Plan with test/verification steps, and link Related Issues.
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.
Title check ❓ Inconclusive The title is partially related to the changeset. It mentions FlagHints and did-you-mean functionality but is incomplete/truncated (missing the final 't' in 'enhancement'), making it slightly unclear. Complete the title to read 'feat(common): add FlagHints + did-you-mean unknown-flag error enhancement' for clarity and professionalism.
✅ Passed checks (2 passed)
Check name Status Explanation
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 •
edited
Loading

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

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

@xzcong0820
fix(common): use validation_error type for priority-3/4 flag error paths
b06ba16 
wrapFlagError priority-3 (no match) and priority-4 (non-unknown-flag error)
were calling output.ErrValidation which hardcodes type="validation"; tech-design
§处理优先级 lines 115/116 require type="validation_error".

Replace both callsites with output.Errorf(output.ExitValidation, "validation_error", msg)
so the type string matches the spec without touching the global ErrValidation helper.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@xzcong0820