perf(im): parallelize thread_replies fetches with bounded concurrency

[sammi-bytedance]

Summary

ExpandThreadReplies previously issued a strictly-serial GET per unique thread_id. Each call is 1s RTT, so chats with N thread roots paid an N×1s stall before the main message output could render — the same latency-multiplier pattern that motivated #1146 (reactions). This PR parallelizes the per-thread fetches with the same bounded fan-out pattern and consolidates the follow-up sender-name resolution into a single batched contact API call. Scope is limited to convert_lib/thread.go + its test.

Changes

• Phase 1 — plan + concurrent fetch. Walk messages in first-seen order to enumerate unique thread_ids and allocate a per-thread limit greedily from the remaining totalLimit budget (threads past the budget skip outright, preserving the existing semantic). Dispatch the planned fetchThreadReplies calls under bounded concurrency (cap = 4 in flight, same as the reactions PR). Each goroutine writes only to its own results[i] slot — no shared mutable state.
• Phase 2 — sequential attach. Stays single-threaded because FormatMessageItem may trigger a merge_forward sub-message GET that writes to the shared nameCache. Phase 2 is now split into three sub-steps:
  • 2a) FormatMessageItem each thread's raw replies.
  • 2b) One batched ResolveSenderNames across all prepared replies (was previously called once per thread, potentially firing a contact API request per thread that introduced a new sender).
  • 2c) AttachSenderNames + write thread_replies onto each plan's host outer message (the first message that referenced that thread_id — preserves existing duplicate handling).
• Semantics preserved. First outer message per thread_id is still the host. totalLimit budget honored (with a documented mild pessimism: budget is allocated based on the planned per-thread limit, not the actual returned count). thread_replies_error: true on fetch failure unchanged. thread_has_more: true propagated unchanged.

Test Plan

• All three pre-existing tests pass unchanged (TestExpandThreadReplies, TestFetchThreadRepliesError, TestExpandThreadRepliesMarksFetchError) — they exercise the single-thread fast path.

• New TestExpandThreadRepliesMultiThreadConcurrent: 5 distinct thread roots → 5 planned fetches under the bounded fan-out. Each thread's reply is tagged with its own thread_id so a goroutine cross-contamination bug would manifest as mis-attached replies. Assertion verifies callCount=5 and every host received the right reply.

• go test -race -count=1 ./shortcuts/im/convert_lib/ clean.

• gofmt -l . / go vet ./... clean.

• Live lark-cli im +chat-messages-list --chat-id <oc_xxx> --page-size 50 --format json --as user --no-reactions against a real group chat with 4 thread roots:

  variant	--no-reactions median wall time
  before (serial fetches)	14.3s
  after (parallel + batched sender resolve)	12.8s

  Modest delta on this specific chat (~1.5s) because only 4 thread roots → serial path was 4 × ~1s of which parallel saves ~3 of 4. Improvement scales linearly with thread-root count: a chat with 10 thread roots would save proportionally more.

Related Issues

• Orthogonal to perf(im): parallelize reactions, thread_replies, and merge_forward fetches #1146 (reactions parallelization). The two optimizations compose: with both merged, a busy chat's --no-reactions wall time drops further than either alone.
• Out of scope here (separate follow-ups when needed): parallelizing merge_forward sub-message expansion, and fixing the fetchMergeForwardSubMessages error-masking bug (code != 0 with data: null currently surfaces as a generic "empty data" error, hiding the real code / msg / log_id).

Summary by CodeRabbit

• Performance

  • Optimized thread reply fetching with enhanced concurrency handling.

• Bug Fixes

  • Improved error handling for thread reply fetch failures.

• Tests

  • Added test coverage for concurrent multi-thread reply expansion.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7e3be507-4002-4a01-a766-4659e5ea9929

📥 Commits

Reviewing files that changed from the base of the PR and between b91f6a2 and b17ba3d.

📒 Files selected for processing (2)

• shortcuts/im/convert_lib/thread.go
• shortcuts/im/convert_lib/thread_test.go

---

📝 Walkthrough

Walkthrough

ExpandThreadReplies transitions from serial per-thread processing to a two-phase concurrent pipeline: plan and allocate per-thread limits upfront, fetch replies concurrently (max 4 in-flight) with a semaphore, format and resolve names in batch, then attach results. Budgeting now deducts during planning rather than on actual fetches. A new test verifies multi-thread concurrent behavior.

Changes

Thread Reply Expansion Refactoring

Layer / File(s)	Summary
Concurrent thread reply expansion implementation shortcuts/im/convert_lib/thread.go	ExpandThreadReplies refactored into a two-phase pipeline: planning phase enumerates unique thread_ids and allocates per-thread limits from a shared budget; concurrent fetch phase uses a semaphore to cap in-flight fetches at 4 threads; sequential preparation phase formats and batches ResolveSenderNames across all replies; attachment phase writes thread_replies, thread_replies_error, and thread_has_more to host messages. Budgeting semantics changed to deduct limits during planning rather than on actual fetch counts. Error handling now marks host messages with thread_replies_error on fetch failure or nil reply slice.
Multi-thread concurrent expansion test shortcuts/im/convert_lib/thread_test.go	New test TestExpandThreadRepliesMultiThreadConcurrent stubs thread-reply fetches for multiple distinct thread_ids, invokes ExpandThreadReplies with high concurrency cap, asserts exactly one fetch per thread, and verifies replies are attached to correct outer messages by checking returned thread_id values match originating messages.

Sequence Diagram

sequenceDiagram
    participant ER as ExpandThreadReplies
    participant Plan as Planning Phase
    participant Fetch as Concurrent Fetcher
    participant Format as Format & Resolve
    participant Attach as Attachment Phase
    
    ER->>Plan: Enumerate unique thread_ids, allocate per-thread limits
    Plan->>ER: Return planned threads and budget allocation
    ER->>Fetch: Fetch planned thread replies (semaphore cap=4)
    Fetch-->>Fetch: Fetch concurrently across threads
    Fetch->>ER: Return fetched replies per thread
    ER->>Format: Format replies and batch ResolveSenderNames
    Format->>ER: Return formatted & resolved replies
    ER->>Attach: Attach replies, errors, has_more to host messages
    Attach->>ER: Return updated host messages

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A pipeline springs forth, where once threads crawled slow,
Four workers in tandem make replies all glow,
Plan once, fetch fast, then resolve as one song,
Budget stays taut, yet the messages belong. ✨

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[sammi-bytedance]

Folding this into #1146 so both perf changes ship in one PR. Closing in favor of that.