imp(): implement all operations transformations by e11sy · Pull Request #115 · editor-js/document-model

e11sy · 2025-04-11T01:15:00Z

Changes

OperationsBatch now extends Operation

It has own lits of operations
It can transform list of operations against one
It can inverse list of operations

Added OperationType.Neutral - operation, that does not affect model on apply, so could be skipped, this type of operations could appear after transformation
Now Undo/Redo stacks and current OperationsBatch are transformed on event, got from server (with userId !== userId of the current client)
Supported all transformations for Operation
Prevent default browser behaviour on Ctrl + Z to avoid manipulations with browser folders

github-actions · 2025-04-11T01:15:47Z

⏭️ No files to mutate for ./packages/model

github-actions · 2025-04-11T01:16:03Z

⏭️ No files to mutate for ./packages/dom-adapters

github-actions · 2025-04-11T01:16:12Z

Coverage report for `./packages/dom-adapters`

St.^❔	Category	Percentage	Covered / Total
🟢	Statements	100%	0/0
🟢	Branches	100%	0/0
🟢	Functions	100%	0/0
🟢	Lines	100%	0/0

Test suite run success

1 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from 61d4dab

github-actions · 2025-04-11T01:16:25Z

Coverage report for `./packages/model`

St.^❔	Category	Percentage	Covered / Total
🟢	Statements	99.65%	852/855
🟢	Branches	98.7%	228/231
🟢	Functions	98.1%	207/211
🟢	Lines	99.64%	819/822

Test suite run success

441 tests passing in 25 suites.

Report generated by 🧪jest coverage report action from a74d393

github-actions · 2025-04-11T01:16:37Z

Coverage report for `./packages/ot-server`

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	70.97%	22/31
🔴	Branches	20%	1/5
🟡	Functions	75%	6/8
🟡	Lines	68.97%	20/29

Test suite run success

4 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from a74d393

github-actions · 2025-04-11T01:16:50Z

Coverage report for `./packages/collaboration-manager`

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	71.25% (+3.74% 🔼)	228/320
🟡	Branches	77.36% (-2.64% 🔻)	82/106
🟡	Functions	68.18% (+11.04% 🔼)	30/44
🟡	Lines	71.2% (+3.68% 🔼)	225/316

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / getRangesIntersectionType.ts	94.12%	86.67%	100%	94.12%
🟢	OperationsTransformer.ts	91.3%	86.21%	100%	91.3%
🟢	BatchedOperation.ts	88.89%	76.47%	100%	88.57%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟡	CollaborationManager.ts	80% (-3.82% 🔻)	73.08% (-0.84% 🔻)	55.56% (-6.94% 🔻)	79.76% (-4.06% 🔻)

Test suite run success

79 tests passing in 5 suites.

Report generated by 🧪jest coverage report action from a74d393

Copilot

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/collaboration-manager/src/BatchedOperation.ts:13

[nitpick] The file is named 'BatchedOperation.ts' while the exported class is named 'OperationsBatch', which may lead to confusion. Consider renaming either the file or the class so that they are consistent.

export class OperationsBatch<T extends OperationType> extends Operation<T> {

Copilot · 2025-04-11T23:10:46Z

packages/collaboration-manager/src/Operation.ts

+  public transform<K extends OperationType>(againstOp: Operation<K> | Operation<OperationType.Neutral>): Operation<T | OperationType.Neutral> {
+    return this.#transformer.transform(this, againstOp);
  }

  /**


The transformation logic does not check if the adjusted text range becomes negative. This can cause tests (e.g., expecting null when over-deleting) to fail. Consider adding a validation step in the transformer methods (such as in #transformAgainstTextDelete) to return null if the resulting text range is invalid.

Suggested change

public transform<K extends OperationType>(againstOp: Operation<K> | Operation<OperationType.Neutral>): Operation<T | OperationType.Neutral> {

return this.#transformer.transform(this, againstOp);

}

/**

public transform<K extends OperationType>(againstOp: Operation<K> | Operation<OperationType.Neutral>): Operation<T | OperationType.Neutral> | null {

const transformedOp = this.#transformer.transform(this, againstOp);

// Validate the resulting operation's index or range

if (!this.isValidOperation(transformedOp)) {

return null;

}

return transformedOp;

}

/**

* Validates the operation to ensure its index or range is valid

*

* @param operation - operation to validate

*/

private isValidOperation(operation: Operation<T | OperationType.Neutral> | null): boolean {

if (!operation) {

return false;

}

// Example validation: Ensure index is non-negative

if (operation.index.start < 0 || operation.index.end < 0) {

return false;

}

// Add additional validation logic as needed

return true;

}

/**

neSpecc · 2025-04-11T23:11:23Z

packages/collaboration-manager/src/OperationsTransformer.ts

+export class OperationsTransformer {
+  constructor() {}
+
+  public transform<T extends OperationType>(operation: Operation<T>, againstOp: Operation<OperationType>): Operation<T> | Operation<OperationType.Neutral> {


describe logic in jsdoc

gohabereg · 2025-04-11T23:09:22Z

packages/collaboration-manager/src/BatchedOperation.ts

+ *
+ * Operations are batched on timeout basis or if batch is terminated from the outside
+ */
+export class OperationsBatch<T extends OperationType> extends Operation<T> {


packages/collaboration-manager/src/BatchedOperation.ts

gohabereg · 2025-04-11T23:11:55Z

packages/collaboration-manager/src/BatchedOperation.ts

+   * 
+   * @param opBatch - operation batch to clone
+   */ 
+  public static from<T extends OperationType>(opBatch: OperationsBatch<T>): OperationsBatch<T>;


Static methods should go before instance methods

gohabereg · 2025-04-11T23:12:26Z

packages/collaboration-manager/src/BatchedOperation.ts

+      /**
+       * Every batch should have at least one operation
+       */
+      const batch = new OperationsBatch(opBatchOrJSON.operations.shift()!);


This way the first operation would be copied by reference not by value

gohabereg · 2025-04-11T23:13:06Z

packages/collaboration-manager/src/BatchedOperation.ts

+  /**
+   * Array of operations to batch
+   */
+  operations: (Operation<T> | Operation<OperationType.Neutral>)[] = [];


Modifier is missing

gohabereg · 2025-04-11T23:37:41Z

packages/collaboration-manager/src/OperationsTransformer.ts

+  #transformAgainstTextInsert<T extends OperationType>(operation: Operation<T>, againstOp: Operation<OperationType>): Operation<T> | Operation<OperationType.Neutral> {
+    const newIndexBuilder = new IndexBuilder().from(operation.index);
+
+    const amountOfInsertedCharacters = againstOp.data.payload!.length;


Suggested change

const amountOfInsertedCharacters = againstOp.data.payload!.length;

const insertedLength = againstOp.data.payload!.length;

gohabereg · 2025-04-11T23:38:53Z

packages/collaboration-manager/src/OperationsTransformer.ts

+      newIndexBuilder.addTextRange([operation.index.textRange![0], operation.index.textRange![1] + amountOfInsertedCharacters]);
+    }
+
+    return new Operation(operation.type, newIndexBuilder.build(), operation.data, operation.userId, operation.rev);


const newOp = Operation.from(operation); newOp.index = newIndexBuilder.build(); return newOp;

gohabereg · 2025-04-11T23:42:51Z

packages/collaboration-manager/src/OperationsTransformer.ts

+
+    const deletedRightSide = (againstOp.index.textRange![0] > operation.index.textRange![0])
+      && (againstOp.index.textRange![0] < operation.index.textRange![1])
+      && (againstOp.index.textRange![1] <= operation.index.textRange![1]);


Suggested change

&& (againstOp.index.textRange![1] <= operation.index.textRange![1]);

&& (againstOp.index.textRange![1] >= operation.index.textRange![1]);

gohabereg · 2025-04-11T23:44:05Z

packages/collaboration-manager/src/OperationsTransformer.ts

+     * Cover case 2.1
+     */
+    if (deletedLeftSide) {
+      const deletedFromCurrentOpRange = operation.index.textRange![0] - againstOp.index.textRange![1];


Suggested change

const deletedFromCurrentOpRange = operation.index.textRange![0] - againstOp.index.textRange![1];

const deletedFromCurrentOpRange = againstOp.index.textRange![1] - operation.index.textRange![0];

gohabereg · 2025-04-11T23:50:36Z

packages/collaboration-manager/src/OperationsTransformer.ts

+    if (deletedLeftSide) {
+      const deletedFromCurrentOpRange = operation.index.textRange![0] - againstOp.index.textRange![1];
+
+      newIndexBuilder.addTextRange([againstOp.index.textRange![0], operation.index.textRange![1] - deletedFromCurrentOpRange]);


Suggested change

newIndexBuilder.addTextRange([againstOp.index.textRange![0], operation.index.textRange![1] - deletedFromCurrentOpRange]);

newIndexBuilder.addTextRange([againstOp.index.textRange![0], operation.index.textRange![1] - deletedAmount]);

gohabereg · 2025-04-12T00:04:44Z

packages/collaboration-manager/src/OperationsTransformer.ts

+    if (againstIndex.isBlockIndex && currentIndex.blockIndex !== undefined) {
+      /**
+       * Check that againstOp affects current operation
+       */
+      if (againstIndex.blockIndex! <= currentIndex.blockIndex) {
+        return this.#transformAgainstBlockOperation(operation, againstOp);
+      }
+    }
+
+    /**
+     * Cover 4 case
+     * 
+     * Check that againstOp is a text operation and current operation is also a text operation
+     */
+    if (againstIndex.isTextIndex && currentIndex.isTextIndex) {
+      /**
+       * Check that againstOp affects current operation (text operation on the same block and same input)
+       * and against op happened on the left side or has overlapping range
+       */
+      if (currentIndex.dataKey === againstIndex.dataKey && currentIndex.blockIndex === againstIndex.blockIndex && againstIndex.textRange![0] <= currentIndex.textRange![0]) {


Replace with

switch (true) { case againstOp.index.isBlockIndex: return this.#transformAgainsBlockOperation(...); case againstOp.index.isTextIndex: return this.#transformAgainsTextOperation(...); default: return Operation.from(operation); };

And move other conditions into the respective methods

gohabereg · 2025-05-02T15:25:50Z

packages/collaboration-manager/src/CollaborationManager.ts

+    if (operation instanceof BatchedOperation) {
+      operation.operations.forEach((op) => {
+        this.applyOperation(op);
+      });
+    } else {
+      this.applyOperation(operation);
+    }


Better to move this condition into the applyOperation method since BatchedOperation is also an operation

Ah, it's already there. So you can remove it here

gohabereg · 2025-05-02T15:27:37Z

packages/collaboration-manager/src/CollaborationManager.ts

+     * @todo - add debounce timeout 500ms
+     */
+    if (!this.#currentBatch.canAdd(operation)) {
+      this.#undoRedoManager.put(this.#currentBatch);


You can call emptyBatch here to be consistent

gohabereg · 2025-05-02T15:28:31Z

packages/collaboration-manager/src/CollaborationManager.ts

+    /**
+     * If current operation could not be added to the batch, then terminate current batch and create a new one with current operation
+     *
+     * @todo - add debounce timeout 500ms


Would be great if you can implement it in the current PR as this functionality is already in the main branch

gohabereg · 2025-05-02T15:28:56Z

packages/collaboration-manager/src/Operation.spec.ts



 describe('Operation', () => {
-  describe('.transform()', () => {


Don't delete it, those tests still should pass even tho the functionality in another file now

gohabereg · 2025-05-02T15:31:10Z

packages/collaboration-manager/src/CollaborationManager.ts

+  /**
+   * Puts current batch to the undo stack and clears the batch
+   */
+  #emptyBatch(): void {


I don't really like the name as it implies the current batch is being emptied. Also it doesn't show that the current batch is being added to the undo redo manager

gohabereg · 2025-05-02T15:33:48Z

packages/collaboration-manager/src/OperationsTransformer.ts

+       * Cover case 2
+       */
+      case OperationType.Delete:
+        if (operation.index.blockIndex !== undefined) {


You've already checked that on line 90

gohabereg · 2025-05-02T15:35:54Z

packages/collaboration-manager/src/OperationsTransformer.ts

+    if (sameInput && sameBlock && againstIndex.textRange![0] > index.textRange![1]) {
+      return Operation.from(operation);
+    }


You can extend condition here to check if it's not the same input or not the same block

gohabereg · 2025-05-02T15:39:38Z

packages/collaboration-manager/src/OperationsTransformer.ts

+      case (RangeIntersectionType.Right):
+        const overlapLength = index.textRange![1] - againstIndex.textRange![0];
+
+        newIndexBuilder.addTextRange([index.textRange![0], index.textRange![1] - overlapLength]);


You can put againstOp.textRange[0] as end index here

gohabereg · 2025-05-02T15:40:30Z

packages/collaboration-manager/src/UndoRedoManager.ts

+   * @param operation - operation to transform against
+   * @param stack - stack to transform
+   */
+  public transformStack(operation: Operation, stack: Operation[]): void {


Make it pure function please.

Also could be private

gohabereg · 2025-05-02T15:41:41Z

packages/collaboration-manager/src/UndoRedoManager.ts

+   * @param stack - stack to transform
+   */
+  public transformStack(operation: Operation, stack: Operation[]): void {
+    const transformed = stack.flatMap((op) => {


Why flatMap? Transform doesn't return null anymore

gohabereg

Couple of minor comments, looks good to me otherwise

gohabereg · 2025-06-29T22:18:40Z

packages/collaboration-manager/src/BatchedOperation.ts

+       */
+      const batch = new BatchedOperation(Operation.from(opBatchOrJSON.operations[0]));
+
+      opBatchOrJSON.operations.slice(1).forEach((op) => {


Perhaps the linter is not set up, but would be great to have chained calls on the new lines. Here and in the other places

Suggested change

opBatchOrJSON.operations.slice(1).forEach((op) => {

opBatchOrJSON.operations.slice(1)

.forEach((op) => {

gohabereg · 2025-06-29T22:20:22Z

packages/collaboration-manager/src/BatchedOperation.ts

+      return true;
+    }
+
+    if (!op.index.isTextIndex || !lastOp.index.isTextIndex) {


Maybe leave a todo that we might add other index types in the future

gohabereg · 2025-06-29T22:22:48Z

packages/collaboration-manager/src/CollaborationManager.ts

   * Redo last undone operation in the local stack
   */
  public redo(): void {
-    this.#currentBatch?.terminate();


It's not clear why in both cases (undo and redo) there's a call moveBatchToUndo

Maybe putBatchToUndo would be better

gohabereg · 2025-06-29T22:26:12Z

packages/collaboration-manager/src/CollaborationManager.ts

+  #debounce(): void {
+    clearTimeout(this.#debounceTimer);
+
+    this.#debounceTimer = setTimeout(() => {


Please cover untested lines and statements

gohabereg

Missed the most important file

gohabereg · 2025-06-29T22:39:04Z

packages/collaboration-manager/src/OperationsTransformer.ts

+       * Cover case 2
+       */
+      case OperationType.Delete:
+        if (againstOp.index.blockIndex! >= operation.index.blockIndex!) {


Could be just ===, you've checked > on line 104

gohabereg · 2025-06-29T22:44:46Z

packages/collaboration-manager/src/OperationsTransformer.ts

+      /**
+       * Cover case 2.1
+       */
+      case (RangeIntersectionType.Left):


intersectionType contains the type of current op compared to againstOp. So with RageIntersectionType.Left the current op is on the left — the against op is on the right. It should be RangeIntersectionType.Right I guess

gohabereg · 2025-06-29T22:45:07Z

packages/collaboration-manager/src/OperationsTransformer.ts

+      /**
+       * Cover case 2.2
+       */
+      case (RangeIntersectionType.Right):


And here should be RangeIntersectionType.Left

You should add more test cases to cover all of that

imp(): implement all operations transformations

9357cba

feat(collab): add operations transformer class

7a5a0e4

neSpecc requested a review from Copilot April 11, 2025 23:10

Copilot AI reviewed Apr 11, 2025

View reviewed changes

neSpecc reviewed Apr 11, 2025

View reviewed changes

gohabereg reviewed Apr 11, 2025

View reviewed changes

gohabereg reviewed Apr 12, 2025

View reviewed changes

e11sy added 10 commits April 15, 2025 15:59

feat(collaboration-manager): determine intersection type util

3a48624

chore(): renaming

f2be325

imp(collaboration-manager): operationsTransformer improved

2a339e1

imp(batchedOp): improvements

59e8c4f

test(collaboration-manager): add tests for transformations

bb68221

chore(collaboration-manager): clean up

fc106f6

Merge branch 'main' into format-stacks-on-remote-op

88c77a1

test(batchedOp): fix test indexes

8e7f12b

fix (OT-server): tests

afd14a4

chore(): lint fix

0093cea

gohabereg reviewed May 2, 2025

View reviewed changes

e11sy added 4 commits June 15, 2025 17:11

test: recover operation tests

a8c2de8

imp(UndoRedoManager): stacks transformation logic improved

2647a7e

feat: add debounce clear batch timer

61d4dab

fix: lint

a74d393

gohabereg reviewed Jun 29, 2025

View reviewed changes

gohabereg requested changes Jun 29, 2025

View reviewed changes

-  public transform<K extends OperationType>(againstOp: Operation<K> | Operation<OperationType.Neutral>): Operation<T | OperationType.Neutral> {
-    return this.#transformer.transform(this, againstOp);
-  }
-  /**
+  public transform<K extends OperationType>(againstOp: Operation<K> | Operation<OperationType.Neutral>): Operation<T | OperationType.Neutral> | null {
+    const transformedOp = this.#transformer.transform(this, againstOp);
+    // Validate the resulting operation's index or range
+    if (!this.isValidOperation(transformedOp)) {
+      return null;
+    }
+    return transformedOp;
+  }
+  /**
+   * Validates the operation to ensure its index or range is valid
+   *
+   * @param operation - operation to validate
+   */
+  private isValidOperation(operation: Operation<T | OperationType.Neutral> | null): boolean {
+    if (!operation) {
+      return false;
+    }
+    // Example validation: Ensure index is non-negative
+    if (operation.index.start < 0 || operation.index.end < 0) {
+      return false;
+    }
+    // Add additional validation logic as needed
+    return true;
+  }
+  /**

	const amountOfInsertedCharacters = againstOp.data.payload!.length;
	const insertedLength = againstOp.data.payload!.length;

	&& (againstOp.index.textRange![1] <= operation.index.textRange![1]);
	&& (againstOp.index.textRange![1] >= operation.index.textRange![1]);

	const deletedFromCurrentOpRange = operation.index.textRange![0] - againstOp.index.textRange![1];
	const deletedFromCurrentOpRange = againstOp.index.textRange![1] - operation.index.textRange![0];

	newIndexBuilder.addTextRange([againstOp.index.textRange![0], operation.index.textRange![1] - deletedFromCurrentOpRange]);
	newIndexBuilder.addTextRange([againstOp.index.textRange![0], operation.index.textRange![1] - deletedAmount]);



		describe('Operation', () => {
		describe('.transform()', () => {

	opBatchOrJSON.operations.slice(1).forEach((op) => {
	opBatchOrJSON.operations.slice(1)
	.forEach((op) => {

Conversation

e11sy commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

github-actions bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report for ./packages/dom-adapters

Test suite run success

Uh oh!

github-actions bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report for ./packages/model

Test suite run success

Uh oh!

github-actions bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report for ./packages/ot-server

Test suite run success

Uh oh!

github-actions bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report for ./packages/collaboration-manager

Test suite run success

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

e11sy commented Apr 11, 2025 •

edited

Loading

github-actions bot commented Apr 11, 2025 •

edited

Loading

github-actions bot commented Apr 11, 2025 •

edited

Loading

github-actions bot commented Apr 11, 2025 •

edited

Loading

Coverage report for `./packages/dom-adapters`

github-actions bot commented Apr 11, 2025 •

edited

Loading

Coverage report for `./packages/model`

github-actions bot commented Apr 11, 2025 •

edited

Loading

Coverage report for `./packages/ot-server`

github-actions bot commented Apr 11, 2025 •

edited

Loading

Coverage report for `./packages/collaboration-manager`