fix(web): Uniform random distribution during shuffle (#19902)

feat: better random distribution

web/src/lib/managers/timeline-manager/day-group.svelte.ts

@@ -82,11 +82,6 @@ export class DayGroup {
     return this.viewerAssets[0]?.asset;
   }
 
-  getRandomAsset() {
-    const random = Math.floor(Math.random() * this.viewerAssets.length);
-    return this.viewerAssets[random];
-  }
-
   *assetsIterator(options: { startAsset?: TimelineAsset; direction?: Direction } = {}) {
     const isEarlier = (options?.direction ?? 'earlier') === 'earlier';
     let assetIndex = options?.startAsset

web/src/lib/managers/timeline-manager/month-group.svelte.ts

@@ -233,15 +233,6 @@ export class MonthGroup {
     addContext.changedDayGroups.add(dayGroup);
   }
 
-  getRandomDayGroup() {
-    const random = Math.floor(Math.random() * this.dayGroups.length);
-    return this.dayGroups[random];
-  }
-
-  getRandomAsset() {
-    return this.getRandomDayGroup()?.getRandomAsset()?.asset;
-  }
-
   get viewId() {
     const { year, month } = this.yearMonth;
     return year + '-' + month;

web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts

@@ -580,4 +580,60 @@ describe('TimelineManager', () => {
       expect(timelineManager.getMonthGroupByAssetId(assetOne.id)?.yearMonth.month).toEqual(1);
     });
   });
+
+  describe('getRandomAsset', () => {
+    let timelineManager: TimelineManager;
+    const bucketAssets: Record<string, TimelineAsset[]> = {
+      '2024-03-01T00:00:00.000Z': timelineAssetFactory.buildList(1).map((asset) =>
+        deriveLocalDateTimeFromFileCreatedAt({
+          ...asset,
+          fileCreatedAt: fromISODateTimeUTCToObject('2024-03-01T00:00:00.000Z'),
+        }),
+      ),
+      '2024-02-01T00:00:00.000Z': timelineAssetFactory.buildList(10).map((asset, idx) =>
+        deriveLocalDateTimeFromFileCreatedAt({
+          ...asset,
+          // here we make sure that not all assets are on the first day of the month
+          fileCreatedAt: fromISODateTimeUTCToObject(`2024-02-0${idx < 7 ? 1 : 2}T00:00:00.000Z`),
+        }),
+      ),
+      '2024-01-01T00:00:00.000Z': timelineAssetFactory.buildList(3).map((asset) =>
+        deriveLocalDateTimeFromFileCreatedAt({
+          ...asset,
+          fileCreatedAt: fromISODateTimeUTCToObject('2024-01-01T00:00:00.000Z'),
+        }),
+      ),
+    };
+
+    const bucketAssetsResponse: Record<string, TimeBucketAssetResponseDto> = Object.fromEntries(
+      Object.entries(bucketAssets).map(([key, assets]) => [key, toResponseDto(...assets)]),
+    );
+
+    beforeEach(async () => {
+      timelineManager = new TimelineManager();
+      sdkMock.getTimeBuckets.mockResolvedValue([
+        { count: 1, timeBucket: '2024-03-01' },
+        { count: 10, timeBucket: '2024-02-01' },
+        { count: 3, timeBucket: '2024-01-01' },
+      ]);
+
+      sdkMock.getTimeBucket.mockImplementation(({ timeBucket }) => Promise.resolve(bucketAssetsResponse[timeBucket]));
+      await timelineManager.updateViewport({ width: 1588, height: 0 });
+    });
+
+    it('gets all assets once', async () => {
+      const assetCount = timelineManager.assetCount;
+      expect(assetCount).toBe(14);
+      const discoveredAssets: Set<string> = new Set();
+      for (let idx = 0; idx < assetCount; idx++) {
+        const asset = await timelineManager.getRandomAsset(idx);
+        expect(asset).toBeDefined();
+        const id = asset!.id;
+        expect(discoveredAssets.has(id)).toBeFalsy();
+        discoveredAssets.add(id);
+      }
+
+      expect(discoveredAssets.size).toBe(assetCount);
+    });
+  });
 });

web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts

@@ -451,16 +451,42 @@ export class TimelineManager {
     return monthGroupInfo?.monthGroup;
   }
 
-  async getRandomMonthGroup() {
-    const random = Math.floor(Math.random() * this.months.length);
-    const month = this.months[random];
-    await this.loadMonthGroup(month.yearMonth, { cancelable: false });
-    return month;
-  }
+  // note: the `index` input is expected to be in the range [0, assetCount). This
+  // value can be passed to make the method deterministic, which is mainly useful
+  // for testing.
+  async getRandomAsset(index?: number): Promise<TimelineAsset | undefined> {
+    const randomAssetIndex = index ?? Math.floor(Math.random() * this.assetCount);
+
+    let accumulatedCount = 0;
+
+    let randomMonth: MonthGroup | undefined = undefined;
+    for (const month of this.months) {
+      if (randomAssetIndex < accumulatedCount + month.assetsCount) {
+        randomMonth = month;
+        break;
+      }
+
+      accumulatedCount += month.assetsCount;
+    }
+    if (!randomMonth) {
+      return;
+    }
+    await this.loadMonthGroup(randomMonth.yearMonth, { cancelable: false });
+
+    let randomDay: DayGroup | undefined = undefined;
+    for (const day of randomMonth.dayGroups) {
+      if (randomAssetIndex < accumulatedCount + day.viewerAssets.length) {
+        randomDay = day;
+        break;
+      }
+
+      accumulatedCount += day.viewerAssets.length;
+    }
+    if (!randomDay) {
+      return;
+    }
 
-  async getRandomAsset() {
-    const month = await this.getRandomMonthGroup();
-    return month?.getRandomAsset();
+    return randomDay.viewerAssets[randomAssetIndex - accumulatedCount].asset;
   }
 
   updateAssetOperation(ids: string[], operation: AssetOperation) {