fix(im): return partial mget results in messages search instead of all-or-nothing

[nguyenngothuong]

Problem

In +messages-search, when the batch mget step fails for any single chunk, all message details are discarded and replaced with a bare message_ids list. AI agents lose all message content, sender names, and timestamps.

Root Cause

batchMGetMessages() in shortcuts/im/im_messages_search.go returns nil, err on the first batch failure, and the caller falls back to ID-only output.

With 55 search results split into 2 mget batches (50 + 5), a transient failure on batch 1 loses all 55 messages even though batch 2 would succeed.

Fix

Change batchMGetMessages to best-effort:

• continue on batch failure instead of return nil, err
• Return partial results (successfully fetched batches)
• When all batches fail → empty slice → existing fallback still applies

Also simplified the function signature from ([]interface{}, error) to []interface{ since errors are no longer propagated.

Testing

• Added TestImMessagesSearchMgetPartialResults — 55 IDs in 2 batches, first fails, second succeeds, verifies 5 partial results returned
• All existing tests pass (3/3)
• go vet clean
• gofmt clean

Summary by CodeRabbit

• Bug Fixes
  • Improved IM message search resilience to handle transient API failures gracefully. The search now returns available partial results instead of failing entirely when some message details cannot be retrieved.

[coderabbitai]

📝 Walkthrough

Walkthrough

Message enrichment now treats mget API calls as best-effort. Failed batch requests are skipped rather than aborting the entire step, and the Step 2 caller uses empty message item results to detect the fallback path. A new test validates transient mget failures are retried and partial results are produced.

Changes

Best-effort IM message enrichment

Layer / File(s)	Summary
Best-effort mget implementation shortcuts/im/im_messages_search.go	batchMGetMessages signature removes error return and updates batch processing to skip failed mget calls and continue; Step 2 caller checks for empty message items instead of error returns to trigger ID-only fallback.
Partial results test shortcuts/im/im_messages_search_execute_test.go	New test TestImMessagesSearchMgetPartialResults validates transient mget failure recovery: simulates first mget call failure, verifies retry on second call success, and confirms "5 search result(s)" are reported despite initial mget failure.

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 A patch of hay now survives the storm,
When mget falters, enrichment adapts its form,
No batches lost to a single API call—
Partial hops are better than no hops at all! 🌾

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 specifically describes the main change: converting mget results from all-or-nothing to partial/best-effort handling in messages search.
Description check	✅ Passed	The description covers the problem, root cause, fix details, and testing verification. It follows the repository template structure with clear sections on problem statement, testing approach, and verification status.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

shortcuts/im/im_messages_search_execute_test.go (1)

305-306: ⚡ Quick win

Check errors for consistency with other tests in this file.

Lines 107-113 and 200-206 use t.Fatalf to check io.ReadAll and json.Unmarshal errors. Follow that pattern here for robustness and consistency.

♻️ Proposed fix

 		case strings.Contains(req.URL.Path, "/open-apis/im/v1/chats/batch_query"):
 			var body struct {
 				ChatIDs []string `json:"chat_ids"`
 			}
-			rawBody, _ := io.ReadAll(req.Body)
-			json.Unmarshal(rawBody, &body)
+			rawBody, err := io.ReadAll(req.Body)
+			if err != nil {
+				t.Fatalf("ReadAll() error = %v", err)
+			}
+			if err := json.Unmarshal(rawBody, &body); err != nil {
+				t.Fatalf("json.Unmarshal() error = %v", err)
+			}
 			return shortcutJSONResponse(200, map[string]interface{}{

🤖 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/im/im_messages_search_execute_test.go` around lines 305 - 306,
Replace the unchecked error handling for io.ReadAll and json.Unmarshal with the
same t.Fatalf pattern used elsewhere in this test file: capture the error
returned by io.ReadAll(req.Body) into a variable and call t.Fatalf(...) if err
!= nil, and likewise check the error returned by json.Unmarshal(rawBody, &body)
and call t.Fatalf(...) on error; reference the existing variables rawBody,
req.Body and the json.Unmarshal call so the change matches the style of the
other checks in this file.

🤖 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 `@shortcuts/im/im_messages_search_execute_test.go`:
- Around line 305-306: Replace the unchecked error handling for io.ReadAll and
json.Unmarshal with the same t.Fatalf pattern used elsewhere in this test file:
capture the error returned by io.ReadAll(req.Body) into a variable and call
t.Fatalf(...) if err != nil, and likewise check the error returned by
json.Unmarshal(rawBody, &body) and call t.Fatalf(...) on error; reference the
existing variables rawBody, req.Body and the json.Unmarshal call so the change
matches the style of the other checks in this file.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 724a586e-e5db-4501-9d6d-51081abba3c8

📥 Commits

Reviewing files that changed from the base of the PR and between 7fdf558 and 3d316b6.

📒 Files selected for processing (2)

• shortcuts/im/im_messages_search.go
• shortcuts/im/im_messages_search_execute_test.go