Skip to content

feat(im): unify sort flags into --sort field and --order direction#1302

Merged
YangJunzhou-01 merged 7 commits into
mainfrom
feat/im-sort-flags-unify
Jun 12, 2026
Merged

feat(im): unify sort flags into --sort field and --order direction#1302
YangJunzhou-01 merged 7 commits into
mainfrom
feat/im-sort-flags-unify

Conversation

@luozhixiong01

@luozhixiong01 luozhixiong01 commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

The 4 im query commands had three inconsistent sort conventions and leaked upstream API jargon (ByCreateTimeAsc, member_count_desc) directly to users. This PR unifies them on a single rule — --sort selects a field, --order selects a direction, both from fixed enums — so an agent only ever picks from an enum, never constructs a string. Old flags (--sort-type, --sort-by, and --sort on messages/threads) are kept as hidden silent aliases (no deprecation warning), so existing scripts keep working byte-for-byte.

Two design tradeoffs were reviewed and explicitly accepted (full rationale in the design spec):

  • --sort is overloaded across commands — a field selector on chat-list/chat-search, a hidden direction alias on messages/threads. Accepted because the field-value set and direction-value set are disjoint, the clash is hidden from --help, and every mismatch fails fast (exit 2 / unknown flag) — no silent wrong sort, no compat break. The residual cost is conceptual + a latent naming conflict if those two commands ever gain a sortable field.
  • Hidden aliases stay enum-validated — an invalid value on an old flag still fails fast with an error referencing the (help-hidden) old flag and its old allowed set. Accepted to keep behavior identical to before and never silently mis-sort.

Changes

  • Add shortcuts/im/sort_flags.goaliasFlagValue helper (pure, no IO) implementing the "old flag wins only if new not given" rule
  • shortcuts/im/im_chat_list.go — new --sort (create_time/active_time); old --sort-type → hidden alias; map field → sort_type
  • shortcuts/im/im_chat_messages_list.go — rename --sort--order (asc/desc); old --sort → hidden alias (mapped through, not passed through)
  • shortcuts/im/im_threads_messages_list.go — extract shared buildThreadsMessagesListParams (so DryRun and Execute share one asc/desc→sort_type mapping), then rename --sort--order; old --sort → hidden alias
  • shortcuts/im/im_chat_search.go — new --sort (create_time/update_time/member_count, always descending); old --sort-by → hidden alias; map field → sorter
  • Unit tests for all 4 commands: new-flag mapping, alias byte-equivalence, default regression, new-wins-over-old, invalid-value fail-fast, flag-structure assertions
  • skills/lark-im/references/*.md (4 files) — update flag tables/examples to the unified flags; add CAUTION notes on chat-messages-list (time-axis only) and chat-search (descending only)

Test Plan

  • make unit-test passed
  • validate passed (build / vet / unit / integration)
  • local-eval: sandbox E2E 4/4 passed; skillave 8/11 — the 3 fails are eval-harness design (placeholder IDs + implicit dry-run expectation; one query omits the thread ID), not feature defects; every decision point that tests the unification is green
  • acceptance-reviewer passed (5/5 scenarios + exploratory; silent-alias behavior physically verified, byte-identical alias output, zero deprecation warning)
  • manual verification: dry-run matrix across all 4 commands — new flags, old aliases (--sort asc/desc on messages/threads → exit 0, byte-identical upstream), new-wins, invalid-value fail-fast (exit 2), rejected combos (chat-list --order → unknown flag)

Related Issues

N/A

Summary by CodeRabbit

Release Notes

  • New Features

    • Clarified sort flag naming across IM commands: --sort for chat-list (with create_time/active_time), --order for chat-messages-list and threads-messages-list (with asc/desc), and --sort for chat-search (with create_time/update_time/member_count)
    • Backwards compatibility maintained through hidden flag aliases for previous flag names
  • Documentation

    • Updated documentation for affected IM commands to reflect new sort flag names and valid options

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Wondering what really moved? Review this PR in Change Stack to inspect semantic changes, definitions, and references.

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: afe075ea-8a39-4a75-8426-2abe340755a1

📥 Commits

Reviewing files that changed from the base of the PR and between 8c3cba1 and 7e9d54f.

📒 Files selected for processing (15)
  • shortcuts/im/builders_test.go
  • shortcuts/im/im_chat_list.go
  • shortcuts/im/im_chat_list_test.go
  • shortcuts/im/im_chat_messages_list.go
  • shortcuts/im/im_chat_messages_list_test.go
  • shortcuts/im/im_chat_search.go
  • shortcuts/im/im_chat_search_test.go
  • shortcuts/im/im_threads_messages_list.go
  • shortcuts/im/im_threads_messages_list_test.go
  • shortcuts/im/sort_flags.go
  • shortcuts/im/sort_flags_test.go
  • skills/lark-im/references/lark-im-chat-list.md
  • skills/lark-im/references/lark-im-chat-messages-list.md
  • skills/lark-im/references/lark-im-chat-search.md
  • skills/lark-im/references/lark-im-threads-messages-list.md

📝 Walkthrough

Walkthrough

This PR refactors IM shortcuts' sort/order flag naming and wiring. It introduces an aliasFlagValue helper to support backward-compatible flag renames and applies the pattern across chat-list, chat-messages-list, chat-search, and threads-messages-list shortcuts. Each shortcut hides its old flag as an alias while promoting a new one; comprehensive tests validate mapping, parity, and precedence. Documentation and test expectations are updated accordingly.

Changes

IM shortcut flag alias refactoring

Layer / File(s) Summary
Alias flag value resolution infrastructure
shortcuts/im/sort_flags.go, shortcuts/im/sort_flags_test.go
New aliasFlagValue helper determines when a renamed flag's old name was explicitly set and the new name was not, enabling conditional alias fallback. Table-driven tests cover all flag-setting scenarios.
Chat list sort flag refactoring
shortcuts/im/im_chat_list.go, shortcuts/im/im_chat_list_test.go
--sort becomes visible (default create_time, enum create_time/active_time) and --sort-type becomes hidden alias. buildChatListParams maps --sort to upstream enum and resolves alias fallback. Existing tests migrate to new flag; new tests validate mapping, parity, precedence, and flag surface.
Chat messages list order flag refactoring
shortcuts/im/im_chat_messages_list.go, shortcuts/im/im_chat_messages_list_test.go
--order replaces --sort as visible flag (default desc, enum asc/desc); --sort becomes hidden alias. buildChatMessageListRequest derives direction from --order with aliasFlagValue fallback. New test file includes helper, mapping, parity, precedence, and flag-surface tests.
Chat search sort flag refactoring
shortcuts/im/im_chat_search.go, shortcuts/im/im_chat_search_test.go
New visible --sort flag (enum create_time/update_time/member_count); --sort-by hidden as alias. buildSearchChatBody maps --sort to descending sorter with alias override via aliasFlagValue. New test file validates mapping, omission, parity, precedence, and flag surface.
Threads messages list order flag refactoring
shortcuts/im/im_threads_messages_list.go, shortcuts/im/im_threads_messages_list_test.go
--order replaces --sort (default asc, enum asc/desc); --sort hidden as alias. Centralizes query-param construction via buildThreadsMessagesListParams and dry-run formatting via toDryParams. New test file validates mapping, parity, and flag surface.
Documentation and test alignment
shortcuts/im/builders_test.go, skills/lark-im/references/*.md
Updates all IM shortcut documentation to use new flag names and removes old flag references. Adjusts builders_test.go test inputs and expected serialization for page-size and sort-value consistency.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • larksuite/cli#820: Refactored +chat-list sorting flag wiring and dry-run assertions, providing initial context for similar patterns applied across other IM shortcuts.
  • larksuite/cli#1077: Modified im_chat_list.go flag and request-parameter construction, overlapping with the chat-list refactoring checkpoint.

Suggested labels

domain/im, size/M

Suggested reviewers

  • YangJunzhou-01

Poem

🐰 Beneath the CLI's leafy code,
Old flags find rest along the road,
While new ones rise with clearer names,
Aliases keep the backward games,
Through test and doc, the pattern flows—
That's how a flag refactor grows!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: unifying sort flags across IM commands into --sort (field) and --order (direction).
Description check ✅ Passed The description is comprehensive and well-structured, covering summary, detailed changes, thorough test plan with explicit verification steps, and addressing design tradeoffs.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/im-sort-flags-unify
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat/im-sort-flags-unify

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 github-actions Bot added domain/im PR touches the im domain size/M Single-domain feat or fix with limited business impact labels Jun 6, 2026
@luozhixiong01
luozhixiong01 force-pushed the feat/im-sort-flags-unify branch from 7e9d54f to 71a5cd9 Compare June 6, 2026 06:06
@codecov

codecov Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.59259% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.85%. Comparing base (dfa26c3) to head (4d8ec3e).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/im/im_threads_messages_list.go 86.66% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1302      +/-   ##
==========================================
+ Coverage   72.75%   72.85%   +0.09%     
==========================================
  Files         730      732       +2     
  Lines       69034    69120      +86     
==========================================
+ Hits        50228    50354     +126     
+ Misses      15034    14989      -45     
- Partials     3772     3777       +5     

☔ View full report in Codecov by Harness.
📢 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.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#feat/im-sort-flags-unify -y -g

@luozhixiong01
luozhixiong01 force-pushed the feat/im-sort-flags-unify branch from 71a5cd9 to 4d8ec3e Compare June 11, 2026 08:22
@YangJunzhou-01
YangJunzhou-01 merged commit 8e60f01 into main Jun 12, 2026
37 of 41 checks passed
@YangJunzhou-01
YangJunzhou-01 deleted the feat/im-sort-flags-unify branch June 12, 2026 07:27
luozhixiong01 added a commit that referenced this pull request Jun 16, 2026
Rebased onto main v1.0.54, which added IM content after this branch forked.
Fold main's new facts into the slimmed/gotcha-only form and fix drift:

- SKILL.md: compact the newly-merged chat.user_setting / chat.managers /
  chat.moderation API resources to the api_compact `action`(identity) form
  (consistent with the rest; regen-equivalent), and note the opt-in
  `--download-resources` flag on the three message-pulling shortcuts.
- chat-search: fix stale `--sort-by` -> `--sort`, add `--chat-modes`, and
  fold the "`--sort` is always descending" caution (#1302/#1317).
- chat-messages-list: note `--order` is the only sort axis (no field sort).
- messages-send: split @mention by message type; interactive cards are not
  normalized and need card-native `<at>` syntax (#1419).
- flag-cancel: document best-effort double-cancel (feed layer skipped with a
  stderr warning when chat type is undeterminable).
- feed-group-list-item / query-item: p2p cards omit chat_name; resolve via
  chats/batch_query -> p2p_target_id -> contact lookup.
- message-enrichment: add the `--download-resources` contract (merge_forward
  container-id 234003 trap, fail-silent isolation, no extra scope) (#1245).

Docs only; no Go/--help/Desc changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/im PR touches the im domain feature size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants