From 9957a391da5b366bd80eaa57fb95b15e5f268002 Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Tue, 2 Jun 2026 18:04:22 +0000 Subject: [PATCH 1/7] refactor(tree): cleanup move logic --- .../default-schema/defaultEditBuilder.ts | 69 ++++++------ .../defaultChangeFamily.spec.ts | 101 ++++++++++++++++++ .../tree/src/test/shared-tree/editing.spec.ts | 7 -- 3 files changed, 140 insertions(+), 37 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts index aad0fcf42e1c..6de53084d5e6 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts @@ -316,6 +316,7 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild const detachCellId = this.modularBuilder.generateId(count); const attachCellId: CellId = { localId: this.modularBuilder.generateId(count), revision }; if (compareFieldUpPaths(sourceField, destinationField)) { + // The source and destination fields are the same. const change = sequence.changeHandler.editor.move( sourceIndex, count, @@ -331,44 +332,52 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild revision, ); } else { + // The source and destination fields are different. const detachPath = topDownPath(sourceField.parent); const attachPath = topDownPath(destinationField.parent); + /** + * The number of elements, starting from the root, that both paths have in common. + * This defines the lowest common ancestor node (LCA) of the source and destination fields. + */ const sharedDepth = getSharedPrefixLength(detachPath, attachPath); + const fieldOnDetachPathUnderLCA = + detachPath[sharedDepth]?.parentField ?? sourceField.field; + const fieldOnAttachPathUnderLCA = + attachPath[sharedDepth]?.parentField ?? destinationField.field; let adjustedAttachField = destinationField; - // After the above loop, `sharedDepth` is the number of elements, starting from the root, - // that both paths have in common. - if (sharedDepth === detachPath.length) { - const attachField = attachPath[sharedDepth]?.parentField ?? destinationField.field; - if (attachField === sourceField.field) { - // The detach occurs in an ancestor field of the field where the attach occurs. - let attachAncestorIndex = attachPath[sharedDepth]?.parentIndex ?? sourceIndex; - if (attachAncestorIndex < sourceIndex) { - // The attach path runs through a node located before the detached nodes. - // No need to adjust the attach path. - } else if (sourceIndex + count <= attachAncestorIndex) { - // The attach path runs through a node located after the detached nodes. - // adjust the index for the node at that depth of the path, so that it is interpreted correctly - // in the composition performed by `submitChanges`. - attachAncestorIndex -= count; - let parent: UpPath | undefined = attachPath[sharedDepth - 1]; - const parentField = attachPath[sharedDepth] ?? oob(); + // If the attach path runs through the field where the detach occurs... + if ( + sharedDepth === detachPath.length && + fieldOnDetachPathUnderLCA === fieldOnAttachPathUnderLCA + ) { + const firstDifferentAttachAncestor = attachPath[sharedDepth]; + // We know that the attach path runs deeper because both paths would otherwise bottom-out in the same field, + // which is handled in a separate code path above. + assert(firstDifferentAttachAncestor !== undefined, "Unexpected identical paths"); + if (firstDifferentAttachAncestor.parentIndex < sourceIndex) { + // The attach path runs through a node located before the detached nodes. + // No need to adjust the attach path. + } else if (sourceIndex + count <= firstDifferentAttachAncestor.parentIndex) { + // The attach path runs through a node located after the detached nodes. + // Adjust the index for the node at that depth of the path, so that it is interpreted correctly + // in the composition performed by `submitChanges`. + let parent = firstDifferentAttachAncestor.parent; + parent = { + parent, + parentIndex: firstDifferentAttachAncestor.parentIndex - count, + parentField: firstDifferentAttachAncestor.parentField, + }; + for (let i = sharedDepth + 1; i < attachPath.length; i += 1) { parent = { + ...(attachPath[i] ?? oob()), parent, - parentIndex: attachAncestorIndex, - parentField: parentField.parentField, }; - for (let i = sharedDepth + 1; i < attachPath.length; i += 1) { - parent = { - ...(attachPath[i] ?? oob()), - parent, - }; - } - adjustedAttachField = { parent, field: destinationField.field }; - } else { - throw new UsageError( - "Invalid move operation: the destination is located under one of the moved elements. Consider using the Tree.contains API to detect this.", - ); } + adjustedAttachField = { parent, field: destinationField.field }; + } else { + throw new UsageError( + "Invalid move operation: the destination is located under one of the moved elements. Consider using the Tree.contains API to detect this.", + ); } } const moveOut = sequence.changeHandler.editor.moveOut( diff --git a/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts b/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts index f9b55323fb60..9e3dc6b941bd 100644 --- a/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts @@ -824,6 +824,107 @@ describe("DefaultEditBuilder", () => { assert.deepEqual(treeView, [expected]); }); + it("Can move nodes from before an ancestor of the destination field", () => { + const { builder, forest } = initializeEditableForest({ + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [ + { type: brand(numberSchema.identifier), value: 0 }, + { type: brand(numberSchema.identifier), value: 1 }, + { type: brand(numberSchema.identifier), value: 2 }, + { type: brand(numberSchema.identifier), value: 3 }, + { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [{ type: brand(numberSchema.identifier), value: 0 }], + }, + }, + ], + }, + }); + builder.move( + { parent: root, field: fooKey }, + 1, + 3, + { + parent: { + parent: root, + parentField: fooKey, + parentIndex: 4, + }, + field: fooKey, + }, + 1, + ); + const treeView = toJsonableTreeFromForest(forest); + const expected: JsonableTree = { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [ + { type: brand(numberSchema.identifier), value: 0 }, + { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [ + { type: brand(numberSchema.identifier), value: 0 }, + { type: brand(numberSchema.identifier), value: 1 }, + { type: brand(numberSchema.identifier), value: 2 }, + { type: brand(numberSchema.identifier), value: 3 }, + ], + }, + }, + ], + }, + }; + assert.deepEqual(treeView, [expected]); + }); + + it("Can move nodes from after an ancestor of the destination field", () => { + const { builder, forest } = initializeEditableForest({ + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [ + { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [{ type: brand(numberSchema.identifier), value: 0 }], + }, + }, + { type: brand(numberSchema.identifier), value: 1 }, + { type: brand(numberSchema.identifier), value: 2 }, + { type: brand(numberSchema.identifier), value: 3 }, + ], + }, + }); + builder.move( + { parent: root, field: fooKey }, + 1, + 3, + { parent: root_foo0, field: fooKey }, + 1, + ); + const treeView = toJsonableTreeFromForest(forest); + const expected: JsonableTree = { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [ + { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [ + { type: brand(numberSchema.identifier), value: 0 }, + { type: brand(numberSchema.identifier), value: 1 }, + { type: brand(numberSchema.identifier), value: 2 }, + { type: brand(numberSchema.identifier), value: 3 }, + ], + }, + }, + ], + }, + }; + assert.deepEqual(treeView, [expected]); + }); + it("Errors when attempting to move a node under itself", () => { const statingState: JsonableTree = { type: brand(JsonAsTree.JsonObject.identifier), diff --git a/packages/dds/tree/src/test/shared-tree/editing.spec.ts b/packages/dds/tree/src/test/shared-tree/editing.spec.ts index ce32f2c7eb70..95f2829bfb48 100644 --- a/packages/dds/tree/src/test/shared-tree/editing.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/editing.spec.ts @@ -1627,13 +1627,6 @@ describe("Editing", () => { expectJsonTree(tree, expectedState); }); - it("can move a node out from a field and into a field under a sibling", () => { - const tree = makeTreeFromJsonSequence(["A", {}]); - tree.editor.move(rootField, 0, 1, { parent: rootNode2, field: brand("foo") }, 0); - const expectedState: JsonCompatible = [{ foo: "A" }]; - expectJsonTree(tree, expectedState); - }); - it("can rebase a move over the deletion of the source parent", () => { const tree = makeTreeFromJson({ src: ["A", "B"], dst: ["C", "D"] }); const childBranch = tree.fork(); From 4b2409e8b32ccb33a759e549a2b24a4a38aa7b93 Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:43:47 -0700 Subject: [PATCH 2/7] Apply suggestions from code review Co-authored-by: Jason Hartman --- .../default-field-kinds/defaultChangeFamily.spec.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts b/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts index 9e3dc6b941bd..b6f2ae07bc28 100644 --- a/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts @@ -843,9 +843,11 @@ describe("DefaultEditBuilder", () => { }, }); builder.move( + // Path to root.foo. { parent: root, field: fooKey }, 1, 3, + // Path to root.foo[4].foo, which is after the moved nodes in their original location. { parent: { parent: root, @@ -897,9 +899,11 @@ describe("DefaultEditBuilder", () => { }, }); builder.move( + // Path to root.foo. { parent: root, field: fooKey }, 1, 3, + // Path to root.foo[0].foo, which is before the moved nodes in their original location. { parent: root_foo0, field: fooKey }, 1, ); From 4043963bbecf876554a84ccc75fd6c82f55650ae Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Thu, 4 Jun 2026 00:03:53 +0000 Subject: [PATCH 3/7] PR suggestions fom Jason --- .../default-schema/defaultEditBuilder.ts | 61 +++++++++---------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts index 6de53084d5e6..5c2c5fd3d106 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts @@ -340,44 +340,41 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild * This defines the lowest common ancestor node (LCA) of the source and destination fields. */ const sharedDepth = getSharedPrefixLength(detachPath, attachPath); - const fieldOnDetachPathUnderLCA = - detachPath[sharedDepth]?.parentField ?? sourceField.field; - const fieldOnAttachPathUnderLCA = - attachPath[sharedDepth]?.parentField ?? destinationField.field; let adjustedAttachField = destinationField; // If the attach path runs through the field where the detach occurs... - if ( - sharedDepth === detachPath.length && - fieldOnDetachPathUnderLCA === fieldOnAttachPathUnderLCA - ) { - const firstDifferentAttachAncestor = attachPath[sharedDepth]; - // We know that the attach path runs deeper because both paths would otherwise bottom-out in the same field, - // which is handled in a separate code path above. - assert(firstDifferentAttachAncestor !== undefined, "Unexpected identical paths"); - if (firstDifferentAttachAncestor.parentIndex < sourceIndex) { - // The attach path runs through a node located before the detached nodes. - // No need to adjust the attach path. - } else if (sourceIndex + count <= firstDifferentAttachAncestor.parentIndex) { - // The attach path runs through a node located after the detached nodes. - // Adjust the index for the node at that depth of the path, so that it is interpreted correctly - // in the composition performed by `submitChanges`. - let parent = firstDifferentAttachAncestor.parent; - parent = { - parent, - parentIndex: firstDifferentAttachAncestor.parentIndex - count, - parentField: firstDifferentAttachAncestor.parentField, - }; - for (let i = sharedDepth + 1; i < attachPath.length; i += 1) { + if (sharedDepth === detachPath.length) { + const fieldOnDetachPathUnderLCA = sourceField.field; + const fieldOnAttachPathUnderLCA = + attachPath[sharedDepth]?.parentField ?? destinationField.field; + if (fieldOnDetachPathUnderLCA === fieldOnAttachPathUnderLCA) { + // We know that the attach path runs deeper because both paths would otherwise bottom-out in the same field, + // which is handled in a separate code path above. + const firstDifferentAttachAncestor = attachPath[sharedDepth] ?? oob(); + if (firstDifferentAttachAncestor.parentIndex < sourceIndex) { + // The attach path runs through a node located before the detached nodes. + // No need to adjust the attach path. + } else if (sourceIndex + count <= firstDifferentAttachAncestor.parentIndex) { + // The attach path runs through a node located after the detached nodes. + // Adjust the index for the node at that depth of the path, so that it is interpreted correctly + // in the composition performed by `submitChanges`. + let parent = firstDifferentAttachAncestor.parent; parent = { - ...(attachPath[i] ?? oob()), parent, + parentIndex: firstDifferentAttachAncestor.parentIndex - count, + parentField: firstDifferentAttachAncestor.parentField, }; + for (let i = sharedDepth + 1; i < attachPath.length; i += 1) { + parent = { + ...(attachPath[i] ?? oob()), + parent, + }; + } + adjustedAttachField = { parent, field: destinationField.field }; + } else { + throw new UsageError( + "Invalid move operation: the destination is located under one of the moved elements. Consider using the Tree.contains API to detect this.", + ); } - adjustedAttachField = { parent, field: destinationField.field }; - } else { - throw new UsageError( - "Invalid move operation: the destination is located under one of the moved elements. Consider using the Tree.contains API to detect this.", - ); } } const moveOut = sequence.changeHandler.editor.moveOut( From c79f1756e7ef0b8dbe94815ef526c453967a2eff Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Thu, 4 Jun 2026 00:04:57 +0000 Subject: [PATCH 4/7] Remove confusing comment --- .../src/feature-libraries/default-schema/defaultEditBuilder.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts index 5c2c5fd3d106..e74db964896f 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts @@ -341,7 +341,6 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild */ const sharedDepth = getSharedPrefixLength(detachPath, attachPath); let adjustedAttachField = destinationField; - // If the attach path runs through the field where the detach occurs... if (sharedDepth === detachPath.length) { const fieldOnDetachPathUnderLCA = sourceField.field; const fieldOnAttachPathUnderLCA = From 7d26533879e7ef1725f59b990f380eac4413889c Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Fri, 5 Jun 2026 00:15:41 +0000 Subject: [PATCH 5/7] Add missing test --- .../defaultChangeFamily.spec.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts b/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts index b6f2ae07bc28..a1c547464a93 100644 --- a/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts @@ -732,6 +732,56 @@ describe("DefaultEditBuilder", () => { assert.deepEqual(treeView, [expected]); }); + it("Can move nodes from field to a nested field under a sibling field of the same parent", () => { + const { builder, forest } = initializeEditableForest({ + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [ + { type: brand(numberSchema.identifier), value: 0 }, + { type: brand(numberSchema.identifier), value: 1 }, + { type: brand(numberSchema.identifier), value: 2 }, + { type: brand(numberSchema.identifier), value: 3 }, + ], + bar: [ + { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + bar: [{ type: brand(numberSchema.identifier), value: 0 }], + }, + }, + ], + }, + }); + builder.move( + { parent: root, field: fooKey }, + 1, + 3, + { parent: root_bar0, field: barKey }, + 1, + ); + const treeView = toJsonableTreeFromForest(forest); + const expected: JsonableTree = { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + foo: [{ type: brand(numberSchema.identifier), value: 0 }], + bar: [ + { + type: brand(JsonAsTree.JsonObject.identifier), + fields: { + bar: [ + { type: brand(numberSchema.identifier), value: 0 }, + { type: brand(numberSchema.identifier), value: 1 }, + { type: brand(numberSchema.identifier), value: 2 }, + { type: brand(numberSchema.identifier), value: 3 }, + ], + }, + }, + ], + }, + }; + assert.deepEqual(treeView, [expected]); + }); + it("Can move nodes before an ancestor of the moved node", () => { const { builder, forest } = initializeEditableForest({ type: brand(JsonAsTree.JsonObject.identifier), From 0addc4916d4b8348df17bc20e29d0bb1070bbb6e Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Fri, 5 Jun 2026 00:15:51 +0000 Subject: [PATCH 6/7] Apply suggestions --- .../default-schema/defaultEditBuilder.ts | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts index e74db964896f..c8d622c1d1ab 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts @@ -341,14 +341,25 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild */ const sharedDepth = getSharedPrefixLength(detachPath, attachPath); let adjustedAttachField = destinationField; + // Check that the detach parent (location of detach) is along the + // attach path, and thus might need to be adjusted. + // (This is synonymous with checking that the attach parent is below + // or the same as the detach parent.) + // If the attach parent is above the detach parent, then the move + // does not need to be adjusted, as the attach path will not be + // affected by the detach. if (sharedDepth === detachPath.length) { const fieldOnDetachPathUnderLCA = sourceField.field; - const fieldOnAttachPathUnderLCA = - attachPath[sharedDepth]?.parentField ?? destinationField.field; - if (fieldOnDetachPathUnderLCA === fieldOnAttachPathUnderLCA) { - // We know that the attach path runs deeper because both paths would otherwise bottom-out in the same field, - // which is handled in a separate code path above. - const firstDifferentAttachAncestor = attachPath[sharedDepth] ?? oob(); + const firstDifferentAttachAncestor = attachPath[sharedDepth]; + // Check that the detach location uses the same field as in + // the attach path. + // Note that when `firstDifferentAttachAncestor` is undefined, + // that means the parents are the same. Since earlier call + // to `compareFieldUpPaths` returned false, we know that the + // fields must be different AND thus no adjustment is needed. + if (fieldOnDetachPathUnderLCA === firstDifferentAttachAncestor?.parentField) { + // Now working at the level where detach happens along the + // attach path. if (firstDifferentAttachAncestor.parentIndex < sourceIndex) { // The attach path runs through a node located before the detached nodes. // No need to adjust the attach path. @@ -356,12 +367,16 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild // The attach path runs through a node located after the detached nodes. // Adjust the index for the node at that depth of the path, so that it is interpreted correctly // in the composition performed by `submitChanges`. + // Start with path to root that remains intact. let parent = firstDifferentAttachAncestor.parent; + // Extend with index-adjusted parent. parent = { parent, parentIndex: firstDifferentAttachAncestor.parentIndex - count, parentField: firstDifferentAttachAncestor.parentField, }; + // Extend with the rest of the attach path, which is unaffected + // apart from parentage replacements. for (let i = sharedDepth + 1; i < attachPath.length; i += 1) { parent = { ...(attachPath[i] ?? oob()), From 71fcb1a93ee53e528ba395757e7c566bdb6108e5 Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Fri, 5 Jun 2026 00:17:24 +0000 Subject: [PATCH 7/7] Tweaks --- .../default-schema/defaultEditBuilder.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts index c8d622c1d1ab..c5a5d04f4404 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts @@ -341,17 +341,19 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild */ const sharedDepth = getSharedPrefixLength(detachPath, attachPath); let adjustedAttachField = destinationField; - // Check that the detach parent (location of detach) is along the - // attach path, and thus might need to be adjusted. - // (This is synonymous with checking that the attach parent is below - // or the same as the detach parent.) - // If the attach parent is above the detach parent, then the move - // does not need to be adjusted, as the attach path will not be - // affected by the detach. + // Check if the detach parent + // (the node directly above the location of detach) + // is along the attach path, + // in which case the attach path might need to be adjusted. + // This is synonymous with checking if the attach parent + // (the node directly above the location of attach) + // is either below or the same as the detach parent. + // If not, then the move does not need to be adjusted + // as the attach path will not be affected by the detach. if (sharedDepth === detachPath.length) { const fieldOnDetachPathUnderLCA = sourceField.field; const firstDifferentAttachAncestor = attachPath[sharedDepth]; - // Check that the detach location uses the same field as in + // Check if the detach location uses the same field as in // the attach path. // Note that when `firstDifferentAttachAncestor` is undefined, // that means the parents are the same. Since earlier call