✨(collaboration) show a migrated history save by save

A document's imported past is attributed to `system`, and until now it went
through the same grouping as ordinary editing. That is wrong twice over.

It loses history. Before the migration the editor saved the whole document on
an interval of exactly 60s, each save becoming one version of the legacy file,
and the migration replays them at their original timestamps — so an imported
history arrives as a chain of entries spaced almost exactly the width of the
grouping window. Whether two of them survived as two came down to whether the
round trip that wrote them ran a few milliseconds fast or slow: measured over
simulated sessions, about a third of the chain collapsed. A document whose
saves happened closer together lost far more — four versions two seconds apart
became one.

And it is the wrong question to ask of them. Grouping summarises someone's
editing into the moments that are worth listing; there was no editing session
here, only a record of saves that already happened, and the only honest thing
to do with that is to show it one for one.

`system` is now excluded from grouping on both sides. The collaboration server
takes the exclusion as a parameter, which is what the yhub bump in this commit
is for. The panel applies it again and more strictly: the server declines to
merge an excluded entry into the one before it, having already refused on the
author comparison it makes, while the panel merges across authors and so must
refuse in both directions — an edit made moments after a document was migrated
must not absorb the imported entry beside it, or be absorbed by it.

Live editing is unchanged: several edits by one person seconds apart are still
one version.

Signed-off-by: Kevin Jahns <kevin.jahns@protonmail.com>

CHANGELOG.md

@@ -28,9 +28,14 @@ and this project adheres to
   become one, no version spans more than a minute, and changes are merged
   whoever made them, because a version is a moment in the document rather than a
   moment in one person's editing. Selecting one previews the document exactly as
-  it stood then. Note that a document whose history has not yet been replayed
-  into the collaboration server (`manage.py migrate_documents`) lists only the
-  edits made since it moved there
+  it stood then. A migrated document's imported history is the exception to the
+  grouping and is listed save by save: those entries are a record of saves that
+  already happened rather than an editing session to summarise, and they arrive
+  spaced exactly the width of the grouping window, so grouping them would have
+  dropped about a third of them depending on how fast the network was years ago.
+  Note that a document whose history has not yet been replayed into the
+  collaboration server (`manage.py migrate_documents`) lists only the edits made
+  since it moved there
 
 - ✨(frontend) make restoring a version work again. It has done nothing since
   the migration, while still promising that the document would be replaced. The

documentation/collaboration.md

@@ -144,6 +144,14 @@ moment in one person's editing, so two people typing in the same minute produce
 two interleaved ones. The collaboration server only ever groups changes by the same author, so this
 last step happens in the browser, on top of its grouping.
 
+With one exception: **a migrated document's imported history is never grouped.** Before the
+migration the editor saved the whole document once a minute, and the migration replays each of
+those saves at its original timestamp, attributed to `system`. Grouped like ordinary editing they
+would land on the granularity itself — a chain of entries a minute apart, merging or not depending
+on how fast the network was on the day each was written, losing about a third of the chain. There
+was no editing session to summarise here, only a record of saves that already happened, so they are
+shown one for one. Both the collaboration server and the browser are told to leave them alone.
+
 ## Restoring a previous state
 
 Selecting a version and restoring it asks the collaboration server to undo everything that happened

src/frontend/apps/impress/src/features/docs/doc-versioning/__tests__/utils.test.ts

@@ -86,6 +86,47 @@ describe('mergeActivityEntries', () => {
     expect(version.to).toBe(900);
   });
 
+  it('keeps a migrated history intact, save by save', () => {
+    /**
+     * The case `UNGROUPED_AUTHORS` exists for. The backend used to save the
+     * whole document every 60s, each save becoming one version of the legacy
+     * file, and the migration replays them at their original timestamps, all
+     * attributed to `system` — so an imported history is a chain of entries
+     * spaced almost exactly the granularity apart, with nothing to tell them
+     * apart by author.
+     *
+     * Jitter is applied in both directions on purpose: it is the negative side
+     * that would merge, and a perfectly fixed interval would pass either way.
+     */
+    let t = 0;
+    const legacy: ActivityEntry[] = [];
+    [0, -900, 700, -1500, 400, -300, 1200, -80].forEach((jitter) => {
+      legacy.push(entry(t, t, 'system'));
+      t += 60_000 + jitter;
+    });
+
+    expect(mergeActivityEntries(legacy)).toHaveLength(legacy.length);
+  });
+
+  it('never folds an edit into the imported history beside it', () => {
+    // stricter than the collaboration server's own test, which would have
+    // refused this on the author comparison it makes and this one does not: a
+    // document edited moments after it was migrated must not absorb the
+    // migration entry, nor be absorbed by it
+    const versions = mergeActivityEntries([
+      entry(0, 0, 'system'),
+      entry(500, 500, 'alice'),
+      entry(1_000, 1_000, 'system'),
+    ]);
+
+    expect(versions).toHaveLength(3);
+    expect(versions.map((v) => v.by)).toEqual([
+      ['system'],
+      ['alice'],
+      ['system'],
+    ]);
+  });
+
   it('keeps an unattributed change without inventing an author', () => {
     const [version] = mergeActivityEntries([entry(0, 0, null)]);
 

src/frontend/apps/impress/src/features/docs/doc-versioning/api/useDocActivity.tsx

@@ -9,7 +9,11 @@ import {
 import { useCollaborationTarget } from '@/core/config/hooks/useCollaborationUrl';
 
 import { APIActivity, DocVersion } from '../types';
-import { VERSION_GRANULARITY_MS, mergeActivityEntries } from '../utils';
+import {
+  UNGROUPED_AUTHORS,
+  VERSION_GRANULARITY_MS,
+  mergeActivityEntries,
+} from '../utils';
 
 export type DocActivityParam = {
   docId: string;
@@ -41,6 +45,9 @@ const getDocActivity = async (
         group: true,
         groupMaxGap: VERSION_GRANULARITY_MS,
         groupMaxDuration: VERSION_GRANULARITY_MS,
+        // the imported history is shown save by save — see UNGROUPED_AUTHORS.
+        // The panel applies this again, on both sides of an entry.
+        groupExclude: UNGROUPED_AUTHORS.join(','),
       },
     },
   );

src/frontend/apps/impress/src/features/docs/doc-versioning/utils.ts

@@ -11,6 +11,24 @@ import { ActivityEntry, DocVersion } from './types';
  */
 export const VERSION_GRANULARITY_MS = 60_000;
 
+/**
+ * Authors whose changes are never merged into a version with anything else.
+ *
+ * `system` is not a person who edited for a long time; it is what a document's
+ * imported past is attributed to. The backend used to save the whole document
+ * once a minute, each save becoming one version of the legacy file, and the
+ * migration replays those saves at their original timestamps — so an imported
+ * history arrives as a chain of `system` entries spaced almost exactly the
+ * granularity apart. Grouped like ordinary editing, whether two of those
+ * versions survive as two would come down to how fast the network was on the
+ * day they were written, and about a third of the chain would collapse.
+ *
+ * Grouping is a judgement about someone's editing, and there was no editing
+ * here: these entries are the record of saves that already happened, and the
+ * only honest thing to do with them is to show them one for one.
+ */
+export const UNGROUPED_AUTHORS = ['system'];
+
 /**
  * Merge an ascending activity timeline into the versions the panel lists.
  *
@@ -28,6 +46,13 @@ export const VERSION_GRANULARITY_MS = 60_000;
  *
  * Both comparisons are `<`, matching the server's, so entries exactly a minute
  * apart start a new version rather than joining the old one.
+ *
+ * `UNGROUPED_AUTHORS` is honoured on both sides, which is stricter than the
+ * server's own test and has to be: the server only refuses to merge an excluded
+ * entry into the one before it, because it would have refused anyway on the
+ * author test it makes and this does not. Here an excluded entry both starts a
+ * version and closes it, so an edit made moments after a document was migrated
+ * cannot be folded into the imported history it happens to sit next to.
  */
 export const mergeActivityEntries = (
   activity: ActivityEntry[],
@@ -35,12 +60,17 @@ export const mergeActivityEntries = (
 ): DocVersion[] => {
   const versions: DocVersion[] = [];
   const authors: Set<string>[] = [];
+  const isUngrouped = (by: string | null) =>
+    by !== null && UNGROUPED_AUTHORS.includes(by);
 
   activity.forEach((entry) => {
     const last = versions[versions.length - 1];
+    const lastAuthors = authors[authors.length - 1];
 
     if (
       last &&
+      !isUngrouped(entry.by) &&
+      !Array.from(lastAuthors ?? []).some(isUngrouped) &&
       entry.from - last.to < granularityMs &&
       entry.to - last.from < granularityMs
     ) {

src/yhub-server/package-lock.json

@@ -7,7 +7,7 @@
       "name": "yhub-server",
       "dependencies": {
         "@aws-sdk/client-s3": "3.1110.0",
-        "@y/hub": "0.8.0",
+        "@y/hub": "0.8.2",
         "@y/y": "14.0.0-rc.24",
         "jose": "6.2.8"
       },
@@ -504,9 +504,9 @@
       "license": "ISC"
     },
     "node_modules/@y/hub": {
-      "version": "0.8.0",
-      "resolved": "https://registry.npmjs.org/@y/hub/-/hub-0.8.0.tgz",
-      "integrity": "sha512-omBq00oLl7NVmB2qPMlKmTL/6ZTxP8hYZZHOX0RMsSKL+53JlbLJn9WVX04xtXYaM+Q81aLsy1/BRUwBYf4yOw==",
+      "version": "0.8.2",
+      "resolved": "https://registry.npmjs.org/@y/hub/-/hub-0.8.2.tgz",
+      "integrity": "sha512-JMwHMwmnUAp6N+qz2MKaIRp/eDGOYIqGTRnW+aNyFg4nypMnnsjPzx7rs6/uG4RR1IDEKiqr4A5Khta44Q2NPQ==",
       "license": "AGPL-3.0 OR PROPRIETARY",
       "dependencies": {
         "@y-crdt/yn": "^0.1.4",

src/yhub-server/package.json

@@ -10,7 +10,7 @@
   },
   "dependencies": {
     "@aws-sdk/client-s3": "3.1110.0",
-    "@y/hub": "0.8.0",
+    "@y/hub": "0.8.2",
     "@y/y": "14.0.0-rc.24",
     "jose": "6.2.8"
   },