Rename and better document allowsTreeSupersetOf

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

@@ -82,7 +82,7 @@ const optionalIdentifier = brandConst("Optional")<FieldKindIdentifier>();
  */
 export const optional = new FlexFieldKind(optionalIdentifier, Multiplicity.Optional, {
 	changeHandler: optionalChangeHandler,
-	allowsTreeSupersetOf: (types, other) =>
+	allowedMonotonicUpgrade: (types, other) =>
 		(other.kind === sequence.identifier || other.kind === optionalIdentifier) &&
 		allowsTreeSchemaIdentifierSuperset(types, other.types),
 });
@@ -110,7 +110,7 @@ const requiredIdentifier = brandConst("Value")<FieldKindIdentifier>();
  */
 export const required = new FlexFieldKind(requiredIdentifier, Multiplicity.Single, {
 	changeHandler: requiredFieldChangeHandler,
-	allowsTreeSupersetOf: (types, other) =>
+	allowedMonotonicUpgrade: (types, other) =>
 		// By omitting Identifier here,
 		// this is making a policy choice that a schema upgrade cannot be done from required to identifier.
 		// Since an identifier can be upgraded into a required field,
@@ -129,7 +129,7 @@ const sequenceIdentifier = brandConst("Sequence")<FieldKindIdentifier>();
  */
 export const sequence = new FlexFieldKind(sequenceIdentifier, Multiplicity.Sequence, {
 	changeHandler: sequenceFieldChangeHandler,
-	allowsTreeSupersetOf: (types, other) =>
+	allowedMonotonicUpgrade: (types, other) =>
 		other.kind === sequenceIdentifier &&
 		allowsTreeSchemaIdentifierSuperset(types, other.types),
 });
@@ -141,7 +141,7 @@ const identifierFieldIdentifier = brandConst("Identifier")<FieldKindIdentifier>(
  */
 export const identifier = new FlexFieldKind(identifierFieldIdentifier, Multiplicity.Single, {
 	changeHandler: noChangeHandler,
-	allowsTreeSupersetOf: (types, other) =>
+	allowedMonotonicUpgrade: (types, other) =>
 		// Allows upgrading from identifier to required: which way this upgrade is allowed to go is a subjective policy choice.
 		(other.kind === sequence.identifier ||
 			other.kind === requiredIdentifier ||
@@ -184,7 +184,7 @@ export const forbidden = new FlexFieldKind(
 	{
 		changeHandler: noChangeHandler,
 		// All multiplicities other than Value support empty.
-		allowsTreeSupersetOf: (types, other) =>
+		allowedMonotonicUpgrade: (types, other) =>
 			fieldKinds.get(other.kind)?.multiplicity !== Multiplicity.Single,
 	},
 );

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

@@ -134,7 +134,7 @@ export function allowsFieldSuperset(
 ): boolean {
 	return (
 		policy.fieldKinds.get(original.kind) ?? fail(0xb1b /* missing kind */)
-	).allowsFieldSuperset(policy, originalData, original.types, superset);
+	).allowedMonotonicUpgrade(policy, originalData, original.types, superset);
 }
 
 /**

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

@@ -60,12 +60,13 @@ export class FlexFieldKind<
 	}
 
 	/**
-	 * Returns true if and only if `superset` permits a (non-strict) superset of the subtrees
-	 * allowed by field made from `this` and `originalTypes`.
+	 * True if a client should be able to {@link TreeView.upgradeSchema} from a field schema using this field kind and `originalTypes` to `superset`.
+	 * @remarks
 	 *
-	 * TODO: clarify the relationship between this and FieldKindData, and issues with cyclic schema upgrades.
+	 * @privateRemarks
+	 * See also {@link FieldKindOptions.allowedMonotonicUpgrade} for constraints on the implementation.
 	 */
-	public allowsFieldSuperset(
+	public allowedMonotonicUpgrade(
 		policy: SchemaPolicy,
 		originalData: TreeStoredSchema,
 		originalTypes: TreeTypeSet,
@@ -84,7 +85,7 @@ export class FlexFieldKind<
 		if (isNeverField(policy, originalData, superset)) {
 			return false;
 		}
-		return this.options.allowsTreeSupersetOf(originalTypes, superset);
+		return this.options.allowedMonotonicUpgrade(originalTypes, superset);
 	}
 }
 
@@ -101,12 +102,29 @@ export interface FieldKindOptions<TFieldChangeHandler> {
 	readonly changeHandler: TFieldChangeHandler;
 
 	/**
-	 * Returns true if and only if `superset` permits a (non-strict) superset of the subtrees
-	 * allowed by field made from `this` and `originalTypes`.
+	 * True if a client should be able to {@link TreeView.upgradeSchema} from a field schema using this field kind and `originalTypes` to `superset`.
 	 * @remarks
-	 * Used by {@link FlexFieldKind.allowsFieldSuperset}, which handles the `never` cases before calling this.
+	 * Must return false if such an upgrade could violate and invariants of `superset` any document which is compatible with this + `originalTypes`.
+	 *
+	 * Unlike the rest of the FieldKind API, this function may change over time without changing the FieldKind identifier without causing decoherence between clients.
+	 * It has a different set of constraints:
+	 *
+	 * - This must never allow an upgrade that could violate any invariants of any field kind required by any version of client for any document content (that was not already invalid).
+	 * This prevents a schema upgrade from causing a document to become out of schema.
+	 * - The set of implementations of this function across all fields kinds and all client versions must never permit a cycle.
+	 * This prevents applications which simply do an upgrade when possible from being abler to have two clients both upgrade where one is actually a down grade and they cause an infinite loop of schema upgrade edits.
+	 *
+	 * To help maintain these invariants, any cases where the allowed document content does not increase (but is the same so the upgrade is still in schema) must be considered carefully.
+	 * Such cases, if allowed, could lead to cycles if their inverse is also allowed.
+	 * These cases, if supported, can be removed, but if doing so must be documented and still considered when avoiding cycles.
+	 *
+	 * Used by {@link FlexFieldKind.allowedMonotonicUpgrade}, which handles the `never` cases (empty type sets) before calling this: the implementer does not need to handle those.
+	 *
+	 * TODO: this design is rather limiting, and there is some planned work in this area:
+	 * - Provide schema upgrade schema not reliant on this to ensure monotonicity of schema upgrades like AB#7482.
+	 * - This monotonic upgrade scheme can remain useful for features like AB#53604 so it should be kept and refined.
 	 */
-	readonly allowsTreeSupersetOf: (
+	readonly allowedMonotonicUpgrade: (
 		originalTypes: TreeTypeSet,
 		superset: TreeFieldStoredSchema,
 	) => boolean;

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

@@ -169,7 +169,7 @@ function replaceRevisions(
 export const genericFieldKind: FlexFieldKind = new FlexFieldKind(
 	brandConst("ModularEditBuilder.Generic")<FieldKindIdentifier>(),
 	Multiplicity.Sequence,
-	{ changeHandler: genericChangeHandler, allowsTreeSupersetOf: (types, other) => false },
+	{ changeHandler: genericChangeHandler, allowedMonotonicUpgrade: (types, other) => false },
 );
 
 /**

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

@@ -113,5 +113,5 @@ export const valueHandler = {
 export const valueField = new FlexFieldKind(
 	brandConst("Value")<FieldKindIdentifier>(),
 	Multiplicity.Single,
-	{ changeHandler: valueHandler, allowsTreeSupersetOf: (a, b) => false },
+	{ changeHandler: valueHandler, allowedMonotonicUpgrade: (a, b) => false },
 );

packages/dds/tree/src/test/feature-libraries/modular-schema/comparison.spec.ts

@@ -185,7 +185,7 @@ describe("Schema Comparison", () => {
 		updateTreeSchema(repo, emptyTree.name, emptyTree.schema);
 		const identifierField = fieldSchema(FieldKinds.identifier, [emptyTree.name]);
 		{
-			const result = FieldKinds.required.allowsFieldSuperset(
+			const result = FieldKinds.required.allowedMonotonicUpgrade(
 				defaultSchemaPolicy,
 				repo,
 				new Set([emptyTree.name]),
@@ -194,7 +194,7 @@ describe("Schema Comparison", () => {
 			assert.equal(result, false);
 		}
 		{
-			const result = FieldKinds.identifier.allowsFieldSuperset(
+			const result = FieldKinds.identifier.allowedMonotonicUpgrade(
 				defaultSchemaPolicy,
 				repo,
 				new Set([emptyTree.name]),

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

@@ -165,7 +165,7 @@ const singleNodeHandler: FieldChangeHandler<SingleNodeChangeset> = {
 const singleNodeField = new FlexFieldKind(
 	brandConst("SingleNode")<FieldKindIdentifier>(),
 	Multiplicity.Single,
-	{ changeHandler: singleNodeHandler, allowsTreeSupersetOf: (a, b) => false },
+	{ changeHandler: singleNodeHandler, allowedMonotonicUpgrade: (a, b) => false },
 );
 
 export const fieldKindConfiguration: FieldKindConfiguration = new Map<
@@ -1172,7 +1172,7 @@ describe("ModularChangeFamily", () => {
 		} as unknown as FieldChangeHandler<HasRemovedRootsRefs, FieldEditor<HasRemovedRootsRefs>>;
 		const hasRemovedRootsRefsField = new FlexFieldKind(fieldKind, Multiplicity.Single, {
 			changeHandler: handler,
-			allowsTreeSupersetOf: (a, b) => false,
+			allowedMonotonicUpgrade: (a, b) => false,
 		});
 		const mockFieldKinds = new Map([[fieldKind, hasRemovedRootsRefsField]]);