fix: position sorts should dictate the order that files are uploaded (#1449)

| 🚥 Resolves CX-3309, #1405,
#1367 |
| :------------------- |

## 🧰 Changes

With ReadMe Refactored the `position` value that we use in our API is
not _quite_ the same as it was pre-refactored because it dictates the
position that your page should be sorted within the `_order.yaml` file
that we create for you in Git. If `page-2` has a `position` of `2` and
is created _before_ `page-1`, which has a position of `1`, then the
`_order.yaml` file will have `page-2` sorted before `page-1` because it
came in first.

Pre-Refactored these `position` values were stored as the value you had
alongside the page in our database so a file with a `position` of `2`
would forever have that position until it explicitly changed.

The work I've done here is to change the way that rdme batches up its
API requests to ensure that we make these requests in the order of the
`position` value that we have present. If we don't have a `position`
then we'll fall back to the `slug` available, sorting it naturally
(ensuring that `page-10` is not sorted between `page-1 and `page-2`)

Author: erunion

src/lib/syncPagePath.ts

@@ -6,7 +6,6 @@ import path from 'node:path';
 
 import chalk from 'chalk';
 import ora from 'ora';
-import toposort from 'toposort';
 
 import { APIv2Error } from './apiError.js';
 import { oraOptions } from './logger.js';
@@ -203,16 +202,42 @@ async function pushPage(
   }
 }
 
-function byParentPage(
+function numericPosition(data: GuidesOrReferenceRequestSchema): number {
+  const p = data.position;
+  if (typeof p === 'number' && Number.isFinite(p)) return p;
+  if (typeof p === 'string') {
+    const n = Number(p);
+    if (Number.isFinite(n)) return n;
+  }
+
+  return Number.POSITIVE_INFINITY;
+}
+
+function compareUploadPriority(
   left: PageMetadata<GuidesOrReferenceRequestSchema>,
   right: PageMetadata<GuidesOrReferenceRequestSchema>,
-) {
-  return (right.data.parent?.uri ? 1 : 0) - (left.data.parent?.uri ? 1 : 0);
+): number {
+  const leftPos = numericPosition(left.data);
+  const rightPos = numericPosition(right.data);
+  if (leftPos !== rightPos) {
+    // Avoid `Infinity - Infinity` -> `NaN` which would poison sort when both sides do not have a
+    // `position`.
+    return leftPos < rightPos ? -1 : 1;
+  }
+
+  return left.slug.localeCompare(right.slug, undefined, {
+    numeric: true,
+    sensitivity: 'base',
+  });
 }
 
 /**
- * Sorts files based on their `parent.uri` attribute. If a file has a `parent.uri` attribute,
- * it will be sorted after the file it references.
+ * Sort files for uploading.
+ *
+ * Every page is uploaded after its parent as defined by `parent.uri` when present and when the
+ * parent is in the same upload batch. Pages within these batches are then sorted by ascending
+ * `position` when present; ties are broken by natural order of the page `slug` (numeric substrings
+ * compare as numbers, e.g. `page-4` before `page-10`).
  *
  * @see {@link https://github.com/readmeio/rdme/pull/973}
  * @returns An array of sorted PageMetadata objects
@@ -226,17 +251,47 @@ function sortFiles(
     return bySlug;
   }, {});
 
-  const dependencies = Object.values(filesBySlug).reduce<
-    [PageMetadata<GuidesOrReferenceRequestSchema>, PageMetadata<GuidesOrReferenceRequestSchema>][]
-  >((edges, obj) => {
-    if (obj.data.parent?.uri && filesBySlug[obj.data.parent.uri]) {
-      edges.push([filesBySlug[obj.data.parent.uri], filesBySlug[obj.slug]]);
+  const slugs = Object.keys(filesBySlug);
+  const inDegree = new Map<string, number>();
+  const childrenByParent = new Map<string, string[]>();
+
+  for (const obj of Object.values(filesBySlug)) {
+    const parentKey = obj.data.parent?.uri;
+    if (parentKey && filesBySlug[parentKey]) {
+      if (!childrenByParent.has(parentKey)) {
+        childrenByParent.set(parentKey, []);
+      }
+
+      childrenByParent.get(parentKey)!.push(obj.slug);
+      inDegree.set(obj.slug, (inDegree.get(obj.slug) ?? 0) + 1);
+    } else {
+      inDegree.set(obj.slug, 0);
     }
+  }
+
+  const sorted: PageMetadata<GuidesOrReferenceRequestSchema>[] = [];
+  const ready = slugs.filter(slug => (inDegree.get(slug) ?? 0) === 0);
+  ready.sort((a, b) => compareUploadPriority(filesBySlug[a], filesBySlug[b]));
 
-    return edges;
-  }, []);
+  while (ready.length > 0) {
+    const nextSlug = ready.shift()!;
+    sorted.push(filesBySlug[nextSlug]);
+
+    for (const childSlug of childrenByParent.get(nextSlug) ?? []) {
+      const newDeg = (inDegree.get(childSlug) ?? 0) - 1;
+      inDegree.set(childSlug, newDeg);
+      if (newDeg === 0) {
+        ready.push(childSlug);
+        ready.sort((a, b) => compareUploadPriority(filesBySlug[a], filesBySlug[b]));
+      }
+    }
+  }
+
+  if (sorted.length !== files.length) {
+    throw new Error('Cyclic dependency');
+  }
 
-  return toposort.array(files, dependencies);
+  return sorted;
 }
 
 /**
@@ -289,7 +344,7 @@ export default async function syncPagePath(this: APIv2PageUploadCommands): Promi
   const sortedFiles =
     this.route === 'changelogs' || this.route === 'custom_pages'
       ? (unsortedFiles as PageMetadata<PageRequestSchema<typeof this.route>>[])
-      : sortFiles((unsortedFiles as PageMetadata<PageRequestSchema<typeof this.route>>[]).toSorted(byParentPage));
+      : sortFiles(unsortedFiles as PageMetadata<GuidesOrReferenceRequestSchema>[]);
 
   // push the files to ReadMe
   const rawResults: PromiseSettledResult<PushResult>[] = [];

test/__fixtures__/docs/position-order-missing/page-1.md

@@ -0,0 +1,7 @@
+---
+title: 'Page 1 (position #1)'
+category:
+  uri: category-slug
+---
+
+Body

test/__fixtures__/docs/position-order-missing/page-10.md

@@ -0,0 +1,7 @@
+---
+title: 'Page 10 (position #10)'
+category:
+  uri: category-slug
+---
+
+Body

test/__fixtures__/docs/position-order-missing/page-2.md

@@ -0,0 +1,7 @@
+---
+title: 'Page 2 (position #2)'
+category:
+  uri: category-slug
+---
+
+Body

test/__fixtures__/docs/position-order-missing/page-3.md

@@ -0,0 +1,7 @@
+---
+title: 'Page 3 (position #3)'
+category:
+  uri: category-slug
+---
+
+Body

test/__fixtures__/docs/position-order-missing/page-4.md

@@ -0,0 +1,7 @@
+---
+title: 'Page 4 (position #4)'
+category:
+  uri: category-slug
+---
+
+Body

test/__fixtures__/docs/position-order-natural/page-1.md

@@ -0,0 +1,8 @@
+---
+title: 'Page 1 (position #1)'
+category:
+  uri: category-slug
+position: 1
+---
+
+Body

test/__fixtures__/docs/position-order-natural/page-10.md

@@ -0,0 +1,8 @@
+---
+title: 'Page 10 (position #10)'
+category:
+  uri: category-slug
+position: 10
+---
+
+Body

test/__fixtures__/docs/position-order-natural/page-2.md

@@ -0,0 +1,8 @@
+---
+title: 'Page 2 (position #2)'
+category:
+  uri: category-slug
+position: 2
+---
+
+Body

test/__fixtures__/docs/position-order-natural/page-3.md

@@ -0,0 +1,8 @@
+---
+title: 'Page 3 (position #3)'
+category:
+  uri: category-slug
+position: 3
+---
+
+Body