perf(sync): skip download when all ops fit in piggyback response

Optimize sync to reduce round-trips when remote changes are small:

- Increase PIGGYBACK_LIMIT from 100 to 500 ops on server
- Skip download call when server confirms hasMorePiggyback === false
- CRITICAL: Only skip when hasMorePiggyback is explicitly false, not undefined
  (undefined means no upload happened, so we must still download)

This reduces sync from 2 round-trips to 1 when ≤500 ops are pending,
improving perceived sync performance.

Add 5 unit tests for the skip-download optimization logic.

packages/super-sync-server/src/sync/sync.routes.ts

@@ -255,7 +255,7 @@ export const syncRoutes = async (fastify: FastifyInstance): Promise<void> => {
             let newOps: import('./sync.types').ServerOperation[] | undefined;
             let latestSeq: number;
             let hasMorePiggyback = false;
-            const PIGGYBACK_LIMIT = 100;
+            const PIGGYBACK_LIMIT = 500;
 
             if (lastKnownServerSeq !== undefined) {
               const opsResult = await syncService.getOpsSinceWithSeq(
@@ -380,7 +380,7 @@ export const syncRoutes = async (fastify: FastifyInstance): Promise<void> => {
         let newOps: import('./sync.types').ServerOperation[] | undefined;
         let latestSeq: number;
         let hasMorePiggyback = false;
-        const PIGGYBACK_LIMIT = 100;
+        const PIGGYBACK_LIMIT = 500;
 
         if (lastKnownServerSeq !== undefined) {
           // Use atomic read to get ops and latestSeq together

src/app/pfapi/api/sync/sync.service.spec.ts

@@ -931,4 +931,103 @@ describe('SyncService', () => {
       expect(result.conflictData).toBeDefined();
     });
   });
+
+  describe('operation log sync', () => {
+    beforeEach(() => {
+      // Enable operation log sync for the mock provider
+      mockSyncProvider.supportsOperationSync = true;
+    });
+
+    it('should skip download when hasMorePiggyback is false (all ops fit in piggyback)', async () => {
+      // Configure upload to return no more piggyback ops
+      mockOperationLogSyncService.uploadPendingOps.and.returnValue(
+        Promise.resolve({
+          uploadedCount: 5,
+          piggybackedOps: [], // Piggybacked ops processed internally
+          rejectedCount: 0,
+          rejectedOps: [],
+          hasMorePiggyback: false, // No more ops on server
+        }),
+      );
+
+      await service.sync();
+
+      // Upload should be called
+      expect(mockOperationLogSyncService.uploadPendingOps).toHaveBeenCalledWith(
+        mockSyncProvider,
+      );
+      // Download should NOT be called because hasMorePiggyback is false
+      expect(mockOperationLogSyncService.downloadRemoteOps).not.toHaveBeenCalled();
+    });
+
+    it('should call download when hasMorePiggyback is undefined (no upload happened)', async () => {
+      // When nothing was uploaded, hasMorePiggyback is undefined.
+      // We MUST download to check for remote ops.
+      mockOperationLogSyncService.uploadPendingOps.and.returnValue(
+        Promise.resolve({
+          uploadedCount: 0,
+          piggybackedOps: [],
+          rejectedCount: 0,
+          rejectedOps: [],
+          // hasMorePiggyback is undefined - no API call was made
+        }),
+      );
+
+      await service.sync();
+
+      // Download SHOULD be called because hasMorePiggyback is undefined (not explicitly false)
+      expect(mockOperationLogSyncService.downloadRemoteOps).toHaveBeenCalledWith(
+        mockSyncProvider,
+      );
+    });
+
+    it('should call download when hasMorePiggyback is true (more ops on server)', async () => {
+      // Configure upload to indicate more ops available
+      mockOperationLogSyncService.uploadPendingOps.and.returnValue(
+        Promise.resolve({
+          uploadedCount: 5,
+          piggybackedOps: [], // Ops processed internally
+          rejectedCount: 0,
+          rejectedOps: [],
+          hasMorePiggyback: true, // More ops exist on server
+        }),
+      );
+
+      await service.sync();
+
+      // Upload should be called
+      expect(mockOperationLogSyncService.uploadPendingOps).toHaveBeenCalled();
+      // Download SHOULD be called because hasMorePiggyback is true
+      expect(mockOperationLogSyncService.downloadRemoteOps).toHaveBeenCalledWith(
+        mockSyncProvider,
+      );
+    });
+
+    it('should call download when upload returns null (fresh client)', async () => {
+      // Fresh clients return null from upload
+      mockOperationLogSyncService.uploadPendingOps.and.returnValue(Promise.resolve(null));
+
+      await service.sync();
+
+      // Download SHOULD be called for fresh clients
+      expect(mockOperationLogSyncService.downloadRemoteOps).toHaveBeenCalledWith(
+        mockSyncProvider,
+      );
+    });
+
+    it('should return InSync status after operation log sync completes', async () => {
+      mockOperationLogSyncService.uploadPendingOps.and.returnValue(
+        Promise.resolve({
+          uploadedCount: 0,
+          piggybackedOps: [],
+          rejectedCount: 0,
+          rejectedOps: [],
+        }),
+      );
+
+      const result = await service.sync();
+
+      expect(result.status).toBe(SyncStatus.InSync);
+    });
+  });
 });

src/app/pfapi/api/sync/sync.service.ts

@@ -110,8 +110,36 @@ export class SyncService<const MD extends ModelCfgs> {
       if (currentSyncProvider && this._supportsOpLogSync(currentSyncProvider)) {
         const uploadResult =
           await this._operationLogSyncService.uploadPendingOps(currentSyncProvider);
-        const downloadResult =
-          await this._operationLogSyncService.downloadRemoteOps(currentSyncProvider);
+
+        // OPTIMIZATION: Skip download if all remote ops were already piggybacked during upload.
+        // This reduces sync to a single round-trip when remote changes are small (≤500 ops).
+        // When hasMorePiggyback=true, we still need to download remaining ops.
+        let downloadResult: {
+          serverMigrationHandled: boolean;
+          localWinOpsCreated: number;
+          newOpsCount: number;
+        };
+
+        // OPTIMIZATION: Skip download ONLY when server explicitly confirms no more ops.
+        // hasMorePiggyback === false means: we uploaded something AND server returned the flag.
+        // hasMorePiggyback === undefined means: no upload happened (nothing to upload) - MUST download.
+        // hasMorePiggyback === true means: server has more ops beyond the piggyback limit.
+        if (uploadResult && uploadResult.hasMorePiggyback === false) {
+          // Server confirmed all remote ops fit in piggyback - no download needed
+          const opCount = uploadResult.piggybackedOps.length;
+          PFLog.normal(
+            `${SyncService.L}.${this.sync.name}(): All ops piggybacked (${opCount}), skip download`,
+          );
+          downloadResult = {
+            serverMigrationHandled: false,
+            localWinOpsCreated: 0,
+            newOpsCount: 0,
+          };
+        } else {
+          // Need to download remaining ops
+          downloadResult =
+            await this._operationLogSyncService.downloadRemoteOps(currentSyncProvider);
+        }
 
         // Track if we need a re-upload:
         // 1. Server migration created a SYNC_IMPORT that needs uploading