Address PR #793 review comments

- Simplify hasCacheForKey comment in multiGet
- Drop PR refs (#787) from the pre-warm .catch() comment in
  mergeCollectionWithPatches; keep the why (preserve cache-first
  invariant on transient storage read failures)
- Add blank line between the warm/cold parity test and the
  Storage.multiGet-rejects regression test, per Fabio's suggestion
- Simplify the Storage.multiGet-rejects test's intro comment by
  removing the codex review ID and PR number references; keep the
  what (subscribers must still see the merge) and the why
  (otherwise Onyx.mergeCollection rejects)

Author: elirangoshen

lib/OnyxUtils.ts

@@ -347,9 +347,8 @@ function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<
             continue;
         }
 
-        // Use hasCacheForKey, not a truthy check on the value — otherwise cached falsy values
-        // (0, '', false, null) get treated as cache misses and re-fetched from storage, which
-        // can overwrite the warm cached value with a stale storage value via cache.merge().
+        // hasCacheForKey catches cached falsy values (0, '', false, null) as cache hits, which
+        // a truthy check on the value would miss.
         if (cache.hasCacheForKey(key)) {
             dataMap.set(key, cache.get(key) as OnyxValue<TKey>);
             continue;
@@ -1611,11 +1610,9 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
             // missing keys into a single Storage.multiGet call (vs. N parallel get() invocations) and
             // writes the storage values back to cache before resolving.
             const hasColdExistingKey = existingKeys.some((key) => !cache.hasCacheForKey(key));
-            // Swallow pre-warm read failures the same way the previous get()-based pre-warm did
-            // (see get() catch at the bottom of its definition). Without this, a transient
-            // Storage.multiGet rejection would skip cache.merge() + keysChanged() below and
-            // regress the cache-first invariant established in #787 — subscribers would miss
-            // the merge and Onyx.mergeCollection would reject up to the caller.
+            // Swallow pre-warm read failures so a transient Storage.multiGet rejection doesn't
+            // skip the cache.merge() + keysChanged() below. Subscribers still see the merge even
+            // when storage reads fail.
             const prewarmPromise = hasColdExistingKey
                 ? multiGet(existingKeys).catch((err) => Logger.logInfo(`mergeCollectionWithPatches pre-warm failed; proceeding with cache-only merge. Error: ${err}`))
                 : Promise.resolve();

tests/unit/onyxUtilsTest.ts

@@ -1089,13 +1089,11 @@ describe('OnyxUtils', () => {
             expect(warmResult).toEqual(coldResult);
             expect(coldResult).toEqual({value: 'after', extra: 'kept'});
         });
+
         it('preserves cache-first invariant when Storage.multiGet rejects on the slow path', async () => {
-            // Regression for codex review #3302454987 / PR #793. Before the .catch() at the
-            // pre-warm call site, a Storage.multiGet rejection would propagate up and skip
-            // cache.merge() + keysChanged() entirely — subscribers would miss the merge and
-            // Onyx.mergeCollection would reject. The previous get()-based pre-warm swallowed
-            // these errors per-key inside get()'s own .catch(), so the cache-first invariant
-            // from #787 held even on a flaky read.
+            // A Storage.multiGet rejection during pre-warm must not skip the cache.merge() +
+            // keysChanged() that follow. Without the .catch() at the pre-warm call site,
+            // subscribers would miss the merge and Onyx.mergeCollection would reject.
             const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY;
             const coldMemberKey = `${collectionKey}1`;
             const newMemberKey = `${collectionKey}2`;