Constrainted reverts

packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts

@@ -160,8 +160,17 @@ export interface IDefaultEditBuilder {
 		destinationIndex: number,
 	): void;
 
-	// TODO: document
+	/**
+	 * Add a constraint that the node at the given path must exist.
+	 * @param path - The path to the node that must exist.
+	 */
 	addNodeExistsConstraint(path: UpPath): void;
+
+	/**
+	 * Add a constraint that the node at the given path must exist when undoing a change.
+	 * @param path - The path to the node that must exist when undoing a change.
+	 */
+	addUndoNodeExistsConstraint(path: UpPath): void;
 }
 
 /**
@@ -191,6 +200,10 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild
 		this.modularBuilder.addNodeExistsConstraint(path, this.mintRevisionTag());
 	}
 
+	public addUndoNodeExistsConstraint(path: UpPath): void {
+		this.modularBuilder.addOutputNodeExistsConstraint(path, this.mintRevisionTag());
+	}
+
 	public valueField(field: FieldUpPath): ValueFieldEditBuilder<ITreeCursorSynchronous> {
 		return {
 			set: (newContent: ITreeCursorSynchronous): void => {

packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts

@@ -20,6 +20,12 @@ import type { CrossFieldManager } from "./crossFieldQueries.js";
 import type { CrossFieldKeyRange, NodeId } from "./modularChangeTypes.js";
 import type { EncodedNodeChangeset } from "./modularChangeFormat.js";
 
+export type NestedChangesIndices = [
+	NodeId,
+	number | undefined /* inputIndex */,
+	number | undefined /* outputIndex */,
+][];
+
 /**
  * Functionality provided by a field kind which will be composed with other `FieldChangeHandler`s to
  * implement a unified ChangeFamily supporting documents with multiple field kinds.
@@ -75,13 +81,16 @@ export interface FieldChangeHandler<
 	 * @param change - The field change to get the child changes from.
 	 *
 	 * @returns The set of `NodeId`s that correspond to nested changes in the given `change`.
-	 * Each `NodeId` is associated with the index of the node in the field in the input context of the changeset
-	 * (or `undefined` if the node is not attached in the input context).
+	 * Each `NodeId` is associated with the following:
+	 * - index of the node in the field in the input context of the changeset (or `undefined` if the node is not
+	 * attached in the input context).
+	 * - index of the node in the field in the output context of the changeset (or `undefined` if the node is not
+	 * attached in the output context).
 	 * For all returned entries where the index is defined,
 	 * the indices are are ordered from smallest to largest (with no duplicates).
 	 * The returned array is owned by the caller.
 	 */
-	getNestedChanges(change: TChangeset): [NodeId, number | undefined][];
+	getNestedChanges(change: TChangeset): NestedChangesIndices;
 
 	/**
 	 * @returns A list of all cross-field keys contained in the change.

packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts

@@ -17,6 +17,7 @@ import { assert } from "@fluidframework/core-utils/internal";
 import type { CrossFieldManager } from "./crossFieldQueries.js";
 import type {
 	FieldChangeHandler,
+	NestedChangesIndices,
 	NodeChangeComposer,
 	NodeChangePruner,
 	NodeChangeRebaser,
@@ -82,8 +83,9 @@ function compose(
 	return composed;
 }
 
-function getNestedChanges(change: GenericChangeset): [NodeId, number | undefined][] {
-	return change.toArray().map(([index, nodeChange]) => [nodeChange, index]);
+function getNestedChanges(change: GenericChangeset): NestedChangesIndices {
+	// For generic changeset, the indices in the input and output contexts are the same.
+	return change.toArray().map(([index, nodeChange]) => [nodeChange, index, index]);
 }
 
 function rebaseGenericChange(

packages/dds/tree/src/feature-libraries/modular-schema/index.ts

@@ -41,6 +41,7 @@ export {
 	type ToDelta,
 	NodeAttachState,
 	type FieldChangeEncodingContext,
+	type NestedChangesIndices,
 } from "./fieldChangeHandler.js";
 export type {
 	CrossFieldKeyRange,

packages/dds/tree/src/feature-libraries/modular-schema/modularChangeCodecs.ts

@@ -213,7 +213,7 @@ function makeModularChangeCodec(
 		context: FieldChangeEncodingContext,
 	): EncodedNodeChangeset {
 		const encodedChange: EncodedNodeChangeset = {};
-		const { fieldChanges, nodeExistsConstraint } = change;
+		const { fieldChanges, nodeExistsConstraint, outputNodeExistsConstraint } = change;
 
 		if (fieldChanges !== undefined) {
 			encodedChange.fieldChanges = encodeFieldChangesForJsonI(fieldChanges, context);
@@ -223,6 +223,10 @@ function makeModularChangeCodec(
 			encodedChange.nodeExistsConstraint = nodeExistsConstraint;
 		}
 
+		if (outputNodeExistsConstraint !== undefined) {
+			encodedChange.outputNodeExistsConstraint = outputNodeExistsConstraint;
+		}
+
 		return encodedChange;
 	}
 
@@ -299,7 +303,7 @@ function makeModularChangeCodec(
 		idAllocator: IdAllocator,
 	): NodeChangeset {
 		const decodedChange: NodeChangeset = {};
-		const { fieldChanges, nodeExistsConstraint } = encodedChange;
+		const { fieldChanges, nodeExistsConstraint, outputNodeExistsConstraint } = encodedChange;
 
 		if (fieldChanges !== undefined) {
 			decodedChange.fieldChanges = decodeFieldChangesFromJson(
@@ -315,6 +319,10 @@ function makeModularChangeCodec(
 			decodedChange.nodeExistsConstraint = nodeExistsConstraint;
 		}
 
+		if (outputNodeExistsConstraint !== undefined) {
+			decodedChange.outputNodeExistsConstraint = outputNodeExistsConstraint;
+		}
+
 		return decodedChange;
 	}
 

packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts

@@ -649,8 +649,21 @@ export class ModularChangeFamily
 		crossFieldTable: ComposeTable,
 		revisionMetadata: RevisionMetadataSource,
 	): NodeChangeset {
+		// WARNING: this composition logic assumes that we never make compositions of the following form:
+		// change1: a changeset that impact the existence of a node
+		// change2: a node-exists constraint on that node.
+		// This is currently enforced by the fact that constraints which apply to the input context are included first in the composition.
+		// If that weren't the case, we would need to rebase the status of the constraint backward over the changes from change1.
 		const nodeExistsConstraint = change1.nodeExistsConstraint ?? change2.nodeExistsConstraint;
 
+		// WARNING: this composition logic assumes that we never make compositions of the following form:
+		// change1: an output-node-exists constraint on a node
+		// change2: a changeset that impacts the existence of that node
+		// This is currently enforced by the fact that constraints which apply to the output context are included last in the composition.
+		// If that weren't the case, we would need to rebase the status of the constraint forward over the changes from change2.
+		const outputNodeExistsConstraint =
+			change1.outputNodeExistsConstraint ?? change2.outputNodeExistsConstraint;
+
 		const composedFieldChanges = this.composeFieldMaps(
 			change1.fieldChanges,
 			change2.fieldChanges,
@@ -670,6 +683,10 @@ export class ModularChangeFamily
 			composedNodeChange.nodeExistsConstraint = nodeExistsConstraint;
 		}
 
+		if (outputNodeExistsConstraint !== undefined) {
+			composedNodeChange.outputNodeExistsConstraint = outputNodeExistsConstraint;
+		}
+
 		return composedNodeChange;
 	}
 
@@ -835,6 +852,10 @@ export class ModularChangeFamily
 	): NodeChangeset {
 		const inverse: NodeChangeset = {};
 
+		if (change.outputNodeExistsConstraint !== undefined) {
+			inverse.nodeExistsConstraint = change.outputNodeExistsConstraint;
+		}
+
 		if (change.fieldChanges !== undefined) {
 			inverse.fieldChanges = this.invertFieldMap(
 				change.fieldChanges,
@@ -878,8 +899,6 @@ export class ModularChangeFamily
 			fieldsWithUnattachedChild: new Set(),
 		};
 
-		const constraintState = newConstraintState(change.constraintViolationCount ?? 0);
-
 		const getBaseRevisions = (): RevisionTag[] =>
 			revisionInfoFromTaggedChange(over).map((info) => info.revision);
 
@@ -895,7 +914,6 @@ export class ModularChangeFamily
 			crossFieldTable,
 			rebasedNodes,
 			genId,
-			constraintState,
 			rebaseMetadata,
 		);
 
@@ -907,10 +925,16 @@ export class ModularChangeFamily
 			genId,
 		);
 
+		const constraintState = newConstraintState(change.constraintViolationCount ?? 0);
+		const outputConstraintState = newConstraintState(
+			change.outputConstraintViolationCount ?? 0,
+		);
 		this.updateConstraintsForFields(
 			rebasedFields,
 			NodeAttachState.Attached,
+			NodeAttachState.Attached,
 			constraintState,
+			outputConstraintState,
 			rebasedNodes,
 		);
 
@@ -923,6 +947,7 @@ export class ModularChangeFamily
 			maxId: idState.maxId,
 			revisions: change.revisions,
 			constraintViolationCount: constraintState.violationCount,
+			outputConstraintViolationCount: outputConstraintState.violationCount,
 			builds: change.builds,
 			destroys: change.destroys,
 			refreshers: change.refreshers,
@@ -937,7 +962,6 @@ export class ModularChangeFamily
 		crossFieldTable: RebaseTable,
 		rebasedNodes: ChangeAtomIdBTree<NodeChangeset>,
 		genId: IdAllocator,
-		constraintState: ConstraintState,
 		metadata: RebaseRevisionMetadata,
 	): FieldChangeMap {
 		const change = crossFieldTable.newChange;
@@ -960,7 +984,6 @@ export class ModularChangeFamily
 				genId,
 				crossFieldTable,
 				metadata,
-				constraintState,
 			);
 
 			setInChangeAtomIdMap(rebasedNodes, newId, rebasedNode);
@@ -1336,7 +1359,6 @@ export class ModularChangeFamily
 		genId: IdAllocator,
 		crossFieldTable: RebaseTable,
 		revisionMetadata: RebaseRevisionMetadata,
-		constraintState: ConstraintState,
 	): NodeChangeset {
 		const change = nodeChangeFromId(crossFieldTable.newChange.nodeChanges, newId);
 		const over = nodeChangeFromId(crossFieldTable.baseChange.nodeChanges, baseId);
@@ -1365,34 +1387,54 @@ export class ModularChangeFamily
 			rebasedChange.nodeExistsConstraint = change.nodeExistsConstraint;
 		}
 
+		if (change?.outputNodeExistsConstraint !== undefined) {
+			rebasedChange.outputNodeExistsConstraint = change.outputNodeExistsConstraint;
+		}
+
 		setInChangeAtomIdMap(crossFieldTable.baseToRebasedNodeId, baseId, newId);
 		return rebasedChange;
 	}
 
 	private updateConstraintsForFields(
 		fields: FieldChangeMap,
 		parentAttachState: NodeAttachState,
+		parentOutputAttachState: NodeAttachState,
 		constraintState: ConstraintState,
+		outputConstraintState: ConstraintState,
 		nodes: ChangeAtomIdBTree<NodeChangeset>,
 	): void {
 		for (const field of fields.values()) {
 			const handler = getChangeHandler(this.fieldKinds, field.fieldKind);
-			for (const [nodeId, index] of handler.getNestedChanges(field.change)) {
+			for (const [nodeId, index, outputIndex] of handler.getNestedChanges(field.change)) {
 				const isDetached = index === undefined;
 				const attachState =
 					parentAttachState === NodeAttachState.Detached || isDetached
 						? NodeAttachState.Detached
 						: NodeAttachState.Attached;
-				this.updateConstraintsForNode(nodeId, attachState, constraintState, nodes);
+				const isOutputDetached = outputIndex === undefined;
+				const outputAttachState =
+					parentOutputAttachState === NodeAttachState.Detached || isOutputDetached
+						? NodeAttachState.Detached
+						: NodeAttachState.Attached;
+				this.updateConstraintsForNode(
+					nodeId,
+					attachState,
+					outputAttachState,
+					nodes,
+					outputConstraintState,
+					constraintState,
+				);
 			}
 		}
 	}
 
 	private updateConstraintsForNode(
 		nodeId: NodeId,
 		attachState: NodeAttachState,
-		constraintState: ConstraintState,
+		outputAttachState: NodeAttachState,
 		nodes: ChangeAtomIdBTree<NodeChangeset>,
+		outputConstraintState: ConstraintState,
+		constraintState: ConstraintState,
 	): void {
 		const node = nodes.get([nodeId.revision, nodeId.localId]) ?? fail("Unknown node ID");
 		if (node.nodeExistsConstraint !== undefined) {
@@ -1405,9 +1447,26 @@ export class ModularChangeFamily
 				constraintState.violationCount += isNowViolated ? 1 : -1;
 			}
 		}
+		if (node.outputNodeExistsConstraint !== undefined) {
+			const isNowViolated = outputAttachState === NodeAttachState.Detached;
+			if (node.outputNodeExistsConstraint.violated !== isNowViolated) {
+				node.outputNodeExistsConstraint = {
+					...node.outputNodeExistsConstraint,
+					violated: isNowViolated,
+				};
+				outputConstraintState.violationCount += isNowViolated ? 1 : -1;
+			}
+		}
 
 		if (node.fieldChanges !== undefined) {
-			this.updateConstraintsForFields(node.fieldChanges, attachState, constraintState, nodes);
+			this.updateConstraintsForFields(
+				node.fieldChanges,
+				attachState,
+				outputAttachState,
+				constraintState,
+				outputConstraintState,
+				nodes,
+			);
 		}
 	}
 
@@ -2061,7 +2120,11 @@ export function rebaseRevisionMetadataFromInfo(
 }
 
 function isEmptyNodeChangeset(change: NodeChangeset): boolean {
-	return change.fieldChanges === undefined && change.nodeExistsConstraint === undefined;
+	return (
+		change.fieldChanges === undefined &&
+		change.nodeExistsConstraint === undefined &&
+		change.outputNodeExistsConstraint === undefined
+	);
 }
 
 export function getFieldKind(
@@ -2504,6 +2567,7 @@ function makeModularChangeset(
 		maxId: number;
 		revisions?: readonly RevisionInfo[];
 		constraintViolationCount?: number;
+		outputConstraintViolationCount?: number;
 		builds?: ChangeAtomIdBTree<TreeChunk>;
 		destroys?: ChangeAtomIdBTree<number>;
 		refreshers?: ChangeAtomIdBTree<TreeChunk>;
@@ -2528,6 +2592,12 @@ function makeModularChangeset(
 	if (props.constraintViolationCount !== undefined && props.constraintViolationCount > 0) {
 		changeset.constraintViolationCount = props.constraintViolationCount;
 	}
+	if (
+		props.outputConstraintViolationCount !== undefined &&
+		props.outputConstraintViolationCount > 0
+	) {
+		changeset.outputConstraintViolationCount = props.outputConstraintViolationCount;
+	}
 	if (props.builds !== undefined && props.builds.size > 0) {
 		changeset.builds = props.builds;
 	}
@@ -2702,6 +2772,27 @@ export class ModularEditBuilder extends EditBuilder<ModularChangeset> {
 			),
 		);
 	}
+
+	public addOutputNodeExistsConstraint(path: UpPath, revision: RevisionTag): void {
+		const nodeChange: NodeChangeset = {
+			outputNodeExistsConstraint: { violated: false },
+		};
+
+		this.applyChange(
+			tagChange(
+				buildModularChangesetFromNode({
+					path,
+					nodeChange,
+					nodeChanges: newTupleBTree(),
+					nodeToParent: newTupleBTree(),
+					crossFieldKeys: newCrossFieldKeyTable(),
+					idAllocator: this.idAllocator,
+					revision,
+				}),
+				revision,
+			),
+		);
+	}
 }
 
 function buildModularChangesetFromField(props: {

packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFormat.ts

@@ -67,6 +67,7 @@ export const EncodedNodeChangeset = Type.Object(
 	{
 		fieldChanges: Type.Optional(EncodedFieldChangeMap),
 		nodeExistsConstraint: Type.Optional(EncodedNodeExistsConstraint),
+		outputNodeExistsConstraint: Type.Optional(EncodedNodeExistsConstraint),
 	},
 	noAdditionalProps,
 );