Save obliterate sequence number on zero-length obliterates (#23011)

This PR fixes how we save the obliterate sequence number on the
associated `pendingSegmentGroup`. Previously, the sequence number would
never be set for zero-length obliterates, so we would get stuck
considering the obliterate as unsequenced.

This PR also surfaces zero-length obliterates in
`mergeTreeDeltaCallback` as long as the obliterate's endpoints are not
equal.

packages/dds/merge-tree/src/mergeTree.ts

@@ -193,8 +193,7 @@ function ackSegment(
 			segment.localMovedSeq = obliterateInfo.localSeq = undefined;
 			const seqIdx = moveInfo.movedSeqs.indexOf(UnassignedSequenceNumber);
 			assert(seqIdx !== -1, 0x86f /* expected movedSeqs to contain unacked seq */);
-			moveInfo.movedSeqs[seqIdx] = obliterateInfo.seq =
-				opArgs.sequencedMessage!.sequenceNumber;
+			moveInfo.movedSeqs[seqIdx] = opArgs.sequencedMessage!.sequenceNumber;
 
 			if (moveInfo.movedSeq === UnassignedSequenceNumber) {
 				moveInfo.movedSeq = opArgs.sequencedMessage!.sequenceNumber;
@@ -1317,11 +1316,9 @@ export class MergeTree {
 				});
 			});
 
-			if (
-				opArgs.op.type === MergeTreeDeltaType.OBLITERATE ||
-				opArgs.op.type === MergeTreeDeltaType.OBLITERATE_SIDED
-			) {
-				this.obliterates.addOrUpdate(pendingSegmentGroup.obliterateInfo!);
+			if (pendingSegmentGroup.obliterateInfo !== undefined) {
+				pendingSegmentGroup.obliterateInfo.seq = seq;
+				this.obliterates.addOrUpdate(pendingSegmentGroup.obliterateInfo);
 			}
 
 			// Perform slides after all segments have been acked, so that
@@ -2157,7 +2154,7 @@ export class MergeTree {
 
 		this.slideAckedRemovedSegmentReferences(localOverlapWithRefs);
 		// opArgs == undefined => test code
-		if (movedSegments.length > 0) {
+		if (start.pos !== end.pos || start.side !== end.side) {
 			this.mergeTreeDeltaCallback?.(opArgs, {
 				operation: MergeTreeDeltaType.OBLITERATE,
 				deltaSegments: movedSegments,

packages/dds/sequence/src/sequence.ts

@@ -541,7 +541,9 @@ export abstract class SharedSegmentSequence<T extends ISegment>
 			if (event.isLocal) {
 				this.submitSequenceMessage(opArgs.op);
 			}
+			if (deltaArgs.deltaSegments.length > 0) {
 				this.emit("sequenceDelta", event, this);
+			}
 		});
 
 		this.client.on("maintenance", (args, opArgs) => {

packages/dds/sequence/src/sequenceDeltaEvent.ts

@@ -13,6 +13,7 @@ import {
 	ISegment,
 	MergeTreeDeltaOperationType,
 	MergeTreeDeltaOperationTypes,
+	MergeTreeDeltaType,
 	MergeTreeMaintenanceType,
 	PropertySet, // eslint-disable-next-line import/no-deprecated
 	SortedSegmentSet,
@@ -76,11 +77,16 @@ export abstract class SequenceEventClass<
 		public readonly deltaArgs: IMergeTreeDeltaCallbackArgs<TOperation>,
 		// eslint-disable-next-line import/no-deprecated
 		private readonly mergeTreeClient: Client,
+	) {
+		if (
+			deltaArgs.operation !== MergeTreeDeltaType.OBLITERATE &&
+			deltaArgs.operation !== MergeTreeMaintenanceType.ACKNOWLEDGED
 		) {
 			assert(
 				deltaArgs.deltaSegments.length > 0,
 				0x2d8 /* "Empty change event should not be emitted." */,
 			);
+		}
 		this.deltaOperation = deltaArgs.operation;
 		this.isLocal = opArgs?.sequencedMessage === undefined;
 

packages/dds/sequence/src/test/sharedString.spec.ts

@@ -887,9 +887,7 @@ describe("Shared String Obliterate", () => {
 		sharedString2.connect(services2);
 	});
 
-	// TODO: this test currently fails with 0x2d8
-	// AB#19722
-	it.skip("zero length obliterate in the middle of the string", () => {
+	it("zero length obliterate in the middle of the string", () => {
 		sharedString.insertText(0, "0123456789");
 		containerRuntimeFactory.processAllMessages();
 		assert.equal(
@@ -909,6 +907,6 @@ describe("Shared String Obliterate", () => {
 			sharedString2.getText(),
 			"end state should be equal after obliterate",
 		);
-		assert.equal(sharedString2.getText(), "01234AAA56789", "obliterate failed");
+		assert.equal(sharedString2.getText(), "01234BBB56789", "obliterate failed");
 	});
 });