Optimize annotation fetch order in notebook

Reduce the perceived loading time in the notebook by fetching
annotations in, roughly, the reverse chronological order in which they
are displayed.

By default `SearchClient` fetches annotations by ascending creation
date. This makes sense in the sidebar as it typically correlates,
roughly, with the order in which the sidebar presents annotations by
default (document location order). For the notebook however this is
suboptimal.

 - Add `sortBy` and `sortOrder` options to `SearchClient` and
   `loadAnnotations` to control fetch order, with the defaults remaining
   the same as before (`created`, `asc`).
 - In `NotebookView` set `sortBy` to `updated` and `sortOrder` to desc
   to fetch annotations in reverse chronological order.

src/sidebar/components/NotebookView.js

@@ -52,6 +52,19 @@ function NotebookView({ loadAnnotationsService }) {
       loadAnnotationsService.load({
         groupId: focusedGroup.id,
         maxResults: 5000,
+
+        // Load annotations in reverse-chronological order because that is how
+        // threads are sorted in the notebook view. By aligning the fetch
+        // order with the thread display order we reduce the changes in visible
+        // content as annotations are loaded. This reduces the amount of time
+        // the user has to wait for the content to load before they can start
+        // reading it.
+        //
+        // Fetching is still suboptimal because we fetch both annotations and
+        // replies together from the backend, but the user initially sees only
+        // the top-level threads.
+        sortBy: 'updated',
+        sortOrder: 'desc',
       });
     }
   }, [loadAnnotationsService, focusedGroup, store]);

src/sidebar/components/test/NotebookView-test.js

@@ -54,7 +54,12 @@ describe('NotebookView', () => {
 
     assert.calledWith(
       fakeLoadAnnotationsService.load,
-      sinon.match({ groupId: 'hallothere', maxResults: 5000 })
+      sinon.match({
+        groupId: 'hallothere',
+        maxResults: 5000,
+        sortBy: 'updated',
+        sortOrder: 'desc',
+      })
     );
     assert.calledWith(fakeStore.setSortKey, 'Newest');
   });

src/sidebar/search-client.js

@@ -4,6 +4,11 @@ import { TinyEmitter } from 'tiny-emitter';
  * @typedef {import('../types/api').Annotation} Annotation
  */
 
+/**
+ * @typedef {'created'|'updated'} SortOrder
+ * @typedef {'asc'|'desc'} SortBy
+ */
+
 /**
  * Client for the Hypothesis search API.
  *
@@ -27,6 +32,9 @@ export default class SearchClient extends TinyEmitter {
    *   annotations, it could cause rendering and network misery in the browser.
    *   When present, do not load annotations if the result set size exceeds
    *   this value.
+   *   @param {SortBy} [options.sortBy] - Together with `sortOrder`, specifies in
+   *     what order annotations are fetched from the backend.
+   *   @param {SortOrder} [options.sortOrder]
    */
   constructor(
     searchFn,
@@ -35,6 +43,8 @@ export default class SearchClient extends TinyEmitter {
       separateReplies = true,
       incremental = true,
       maxResults = null,
+      sortBy = /** @type {SortBy} */ ('created'),
+      sortOrder = /** @type {SortOrder} */ ('asc'),
     } = {}
   ) {
     super();
@@ -43,6 +53,8 @@ export default class SearchClient extends TinyEmitter {
     this._separateReplies = separateReplies;
     this._incremental = incremental;
     this._maxResults = maxResults;
+    this._sortBy = sortBy;
+    this._sortOrder = sortOrder;
 
     this._canceled = false;
     /** @type {Annotation[]} */
@@ -55,8 +67,8 @@ export default class SearchClient extends TinyEmitter {
       {
         limit: this._chunkSize,
         offset: offset,
-        sort: 'created',
-        order: 'asc',
+        sort: this._sortBy,
+        order: this._sortOrder,
         _separate_replies: this._separateReplies,
       },
       query

src/sidebar/services/load-annotations.js

@@ -2,12 +2,23 @@
  * A service for fetching annotations, filtered by document URIs and group.
  */
 
+/**
+ * @typedef {import('../search-client').SortBy} SortBy
+ * @typedef {import('../search-client').SortOrder} SortOrder
+ */
+
 /**
  * @typedef LoadAnnotationOptions
  * @prop {string} groupId
  * @prop {string[]} [uris]
  * @prop {number} [maxResults] - If number of annotations in search results
  *   exceeds this value, do not load annotations (see: `SearchClient`)
+ * @prop {SortBy} [sortBy] - Together with `sortOrder`, this controls in what
+ *   order annotations are loaded. To minimize visible content changing as
+ *   annotations load, `sortBy` and `sortOrder` should be chosen to correlate
+ *   with the expected presentation order of annotations/threads in the current
+ *   view.
+ * @prop {SortOrder} [sortOrder]
  */
 
 import SearchClient from '../search-client';
@@ -46,6 +57,20 @@ export default function loadAnnotationsService(
       incremental: true,
       maxResults: options.maxResults ?? null,
       separateReplies: false,
+
+      // Annotations are fetched in order of creation by default. This is expected
+      // to roughly correspond to the order in which threads end up being sorted
+      // because:
+      //
+      // 1. The default thread sort order in the sidebar is by document location
+      // 2. When users annotate a document, they will tend to annotate content in
+      //    document order. Annotations near the top of the document will
+      //    tend to have earlier creation dates.
+      //
+      // If the backend would allow us to sort on document location, we could do even better.
+
+      sortBy: /** @type {SortBy} */ (options.sortBy ?? 'created'),
+      sortOrder: /** @type {SortOrder} */ (options.sortOrder ?? 'asc'),
     };
 
     searchClient = new SearchClient(api.search, searchOptions);

src/sidebar/services/test/load-annotations-test.js

@@ -13,6 +13,8 @@ class FakeSearchClient extends EventEmitter {
     this.cancel = sinon.stub();
     this.incremental = !!opts.incremental;
     this.separateReplies = !!opts.separateReplies;
+    this.sortBy = opts.sortBy;
+    this.sortOrder = opts.sortOrder;
 
     this.get = sinon.spy(query => {
       if (!query.uri) {
@@ -250,6 +252,29 @@ describe('loadAnnotationsService', () => {
       assert.isFalse(searchClients[0].separateReplies);
     });
 
+    it('loads annotations with default sort order', () => {
+      const svc = createService();
+
+      svc.load({ groupId: fakeGroupId, uris: fakeUris });
+
+      assert.equal(searchClients[0].sortBy, 'created');
+      assert.equal(searchClients[0].sortOrder, 'asc');
+    });
+
+    it('loads annotations with custom sort order', () => {
+      const svc = createService();
+
+      svc.load({
+        groupId: fakeGroupId,
+        uris: fakeUris,
+        sortBy: 'updated',
+        sortOrder: 'desc',
+      });
+
+      assert.equal(searchClients[0].sortBy, 'updated');
+      assert.equal(searchClients[0].sortOrder, 'desc');
+    });
+
     it("cancels previously search client if it's still running", () => {
       const svc = createService();
 

src/sidebar/test/search-client-test.js

@@ -173,4 +173,29 @@ describe('SearchClient', () => {
       });
     });
   });
+
+  it('fetches annotations by earliest creation date if `sortBy` and `sortOrder` not set', async () => {
+    const client = new SearchClient(fakeSearchFn);
+
+    client.get({ uri: 'http://example.com' });
+    await awaitEvent(client, 'end');
+
+    const params = fakeSearchFn.getCall(0).args[0];
+    assert.equal(params.sort, 'created');
+    assert.equal(params.order, 'asc');
+  });
+
+  it('fetches annotations in specified order if `sortBy` and `sortOrder` are set', async () => {
+    const client = new SearchClient(fakeSearchFn, {
+      sortBy: 'updated',
+      sortOrder: 'desc',
+    });
+
+    client.get({ uri: 'http://example.com' });
+    await awaitEvent(client, 'end');
+
+    const params = fakeSearchFn.getCall(0).args[0];
+    assert.equal(params.sort, 'updated');
+    assert.equal(params.order, 'desc');
+  });
 });