Fix instance creation error message (#8759)

newIDE/app/src/EditorFunctions/EditorFunctions.spec.js

@@ -2023,15 +2023,74 @@ describe('editorFunctions', () => {
       expect(getInstancePositions(testScene)).toEqual([]);
     });
 
-    it('reports not-found ids when erasing an unknown instance id', async () => {
-      const result = await putInstances({
-        brush_kind: 'erase',
-        existing_instance_ids: 'does-not-exist',
+    it('fails when erasing an unknown instance id (none found, nothing changed)', async () => {
+      // Bypass the `putInstances` helper, which asserts success — here we
+      // expect a failure so the agent gets a real error signal instead of a
+      // misleading success that could make it retry the same call in a loop.
+      const result = await editorFunctions.put_2d_instances.launchFunction({
+        ...makeFakeLaunchFunctionOptionsWithProject(project),
+        args: {
+          scene_name: 'TestScene',
+          object_name: 'Player',
+          layer_name: '',
+          brush_kind: 'erase',
+          existing_instance_ids: 'does-not-exist',
+        },
       });
 
+      expect(result.success).toBe(false);
+      expect(result.message).toEqual(
+        expect.stringContaining(
+          'None of the specified instance ids were found: does-not-exist'
+        )
+      );
+    });
+
+    it('still succeeds erasing when some ids match and others are unknown', async () => {
+      await putInstances({
+        brush_kind: 'point',
+        brush_position: '100,200',
+        new_instances_count: 1,
+      });
+      const [created] = getInstances(testScene);
+
+      const result = await putInstances({
+        brush_kind: 'erase',
+        existing_instance_ids: `${created.uuid},does-not-exist`,
+      });
+
+      // One id matched (so the call did something and must not fail), the other
+      // is reported as not found.
+      expect(result.message).toEqual(
+        expect.stringContaining('Erased 1 instance')
+      );
       expect(result.message).toEqual(
         expect.stringContaining('Instance ids not found: does-not-exist')
       );
+      expect(getInstances(testScene)).toHaveLength(0);
+    });
+
+    it('fails when no requested instance id is found and nothing is created', async () => {
+      // Bypass the `putInstances` helper, which asserts success — here we
+      // expect a failure so the agent gets a real error signal instead of a
+      // misleading success that could make it retry the same call in a loop.
+      const result = await editorFunctions.put_2d_instances.launchFunction({
+        ...makeFakeLaunchFunctionOptionsWithProject(project),
+        args: {
+          scene_name: 'TestScene',
+          object_name: 'Player',
+          layer_name: '',
+          brush_kind: 'none',
+          existing_instance_ids: 'does-not-exist',
+        },
+      });
+
+      expect(result.success).toBe(false);
+      expect(result.message).toEqual(
+        expect.stringContaining(
+          'None of the specified instance ids were found: does-not-exist'
+        )
+      );
     });
   });
 
@@ -2114,6 +2173,40 @@ describe('editorFunctions', () => {
       return positions.sort((a, b) => a.x - b.x || a.y - b.y || a.z - b.z);
     };
 
+    // Collect full instances (with their persistent uuid) so tests can target
+    // existing instances by id for move/erase, like the real tool does.
+    const getInstances = (
+      scene: gdLayout
+    ): Array<{|
+      uuid: string,
+      x: number,
+      y: number,
+      z: number,
+      layer: string,
+    |}> => {
+      const instances = [];
+      const functor = new gd.InitialInstanceJSFunctor();
+      // $FlowFixMe[cannot-write]
+      functor.invoke = instancePtr => {
+        const instance: gdInitialInstance = gd.wrapPointer(
+          // $FlowFixMe[incompatible-type]
+          instancePtr,
+          gd.InitialInstance
+        );
+        instances.push({
+          uuid: instance.getPersistentUuid(),
+          x: instance.getX(),
+          y: instance.getY(),
+          z: instance.getZ(),
+          layer: instance.getLayer(),
+        });
+      };
+      // $FlowFixMe[incompatible-type]
+      scene.getInitialInstances().iterateOverInstances(functor);
+      functor.delete();
+      return instances;
+    };
+
     const putInstances = async (args: any) => {
       const result = await editorFunctions.put_3d_instances.launchFunction({
         ...makeFakeLaunchFunctionOptionsWithProject(project),
@@ -2172,6 +2265,104 @@ describe('editorFunctions', () => {
       });
     });
 
+    it('fails when no requested instance id is found and nothing is created', async () => {
+      // Bypass the `putInstances` helper, which asserts success — here we
+      // expect a failure so the agent gets a real error signal instead of a
+      // misleading success that could make it retry the same call in a loop.
+      const result = await editorFunctions.put_3d_instances.launchFunction({
+        ...makeFakeLaunchFunctionOptionsWithProject(project),
+        args: {
+          scene_name: 'TestScene',
+          object_name: 'Player',
+          layer_name: '',
+          brush_kind: 'none',
+          existing_instance_ids: 'does-not-exist',
+        },
+      });
+
+      expect(result.success).toBe(false);
+      expect(result.message).toEqual(
+        expect.stringContaining(
+          'None of the specified instance ids were found: does-not-exist'
+        )
+      );
+    });
+
+    it('erases an existing instance by id', async () => {
+      await putInstances({
+        brush_kind: 'point',
+        brush_position: '10,20,30',
+        new_instances_count: 2,
+      });
+      const instances = getInstances(testScene);
+      expect(instances).toHaveLength(2);
+
+      const result = await putInstances({
+        brush_kind: 'erase',
+        existing_instance_ids: instances[0].uuid,
+      });
+
+      expect(result.message).toEqual(
+        expect.stringContaining('Erased 1 instance')
+      );
+      expect(getInstances(testScene)).toHaveLength(1);
+    });
+
+    it('fails when erasing an unknown instance id (none found, nothing changed)', async () => {
+      await putInstances({
+        brush_kind: 'point',
+        brush_position: '10,20,30',
+        new_instances_count: 1,
+      });
+
+      // Bypass the `putInstances` helper, which asserts success — here we
+      // expect a failure so the agent gets a real error signal instead of a
+      // misleading success that could make it retry the same call in a loop.
+      const result = await editorFunctions.put_3d_instances.launchFunction({
+        ...makeFakeLaunchFunctionOptionsWithProject(project),
+        args: {
+          scene_name: 'TestScene',
+          object_name: 'Player',
+          layer_name: '',
+          brush_kind: 'erase',
+          existing_instance_ids: 'does-not-exist',
+        },
+      });
+
+      expect(result.success).toBe(false);
+      expect(result.message).toEqual(
+        expect.stringContaining(
+          'None of the specified instance ids were found: does-not-exist'
+        )
+      );
+      // Nothing was erased: the existing instance is still there.
+      expect(getInstances(testScene)).toHaveLength(1);
+    });
+
+    it('still succeeds erasing when some ids match and others are unknown', async () => {
+      await putInstances({
+        brush_kind: 'point',
+        brush_position: '10,20,30',
+        new_instances_count: 1,
+      });
+      const [created] = getInstances(testScene);
+
+      const result = await putInstances({
+        brush_kind: 'erase',
+        existing_instance_ids: `${created.uuid},does-not-exist`,
+      });
+
+      // One id matched (so the call did something and must not fail), the other
+      // is reported as not found.
+      expect(result.message).toEqual(
+        expect.stringContaining('Erased 1 instance')
+      );
+      expect(result.message).toEqual(
+        expect.stringContaining('Instance ids not found: does-not-exist')
+      );
+      expect(getInstances(testScene)).toHaveLength(0);
+    });
+
     // Note: there is intentionally no grid test here. `grid` is not part of
     // supported3dBrushKinds, so the tool schema prevents the model from ever
     // sending it to put_3d_instances (unlike the 2D variant, which supports it).

newIDE/app/src/EditorFunctions/index.js

@@ -2886,6 +2886,23 @@ const put2dInstances: EditorFunction = {
         }
       });
 
+      // If specific instance ids were requested but none matched (and the brush
+      // did not select anything either), the call erased nothing. Return a
+      // failure so the agent gets a real error signal instead of a misleading
+      // success that could make it retry the same call in a loop.
+      if (
+        instancesToDelete.size === 0 &&
+        notFoundExistingInstanceIds.size > 0
+      ) {
+        return makeGenericFailure(
+          `None of the specified instance ids were found: ${Array.from(
+            notFoundExistingInstanceIds
+          ).join(
+            ', '
+          )}. Nothing was changed. Call \`describe_instances\` to get valid ids (the \`id\` field of each instance), and check the scene and layer names.`
+        );
+      }
+
       instancesToDelete.forEach(instance => {
         initialInstances.removeInstance(instance);
       });
@@ -3275,6 +3292,20 @@ const put2dInstances: EditorFunction = {
       }
 
       if (notFoundExistingInstanceIds.size > 0) {
+        // If NONE of the requested instances were found and nothing new was
+        // created, the call did nothing. Return a failure so the agent gets a
+        // real error signal instead of a misleading success — a success here
+        // can make the agent retry the same (often malformed) call in a loop.
+        if (existingInstanceStates.size === 0 && newInstancesCount === 0) {
+          return makeGenericFailure(
+            `None of the specified instance ids were found: ${Array.from(
+              notFoundExistingInstanceIds
+            ).join(
+              ', '
+            )}. Nothing was changed. Call \`describe_instances\` to get valid ids (the \`id\` field of each instance), and check the scene and layer names.`
+          );
+        }
+
         changes.push(
           `Instance ids not found: ${Array.from(
             notFoundExistingInstanceIds
@@ -3550,6 +3581,23 @@ const put3dInstances: EditorFunction = {
         }
       });
 
+      // If specific instance ids were requested but none matched (and the brush
+      // did not select anything either), the call erased nothing. Return a
+      // failure so the agent gets a real error signal instead of a misleading
+      // success that could make it retry the same call in a loop.
+      if (
+        instancesToDelete.size === 0 &&
+        notFoundExistingInstanceIds.size > 0
+      ) {
+        return makeGenericFailure(
+          `None of the specified instance ids were found: ${Array.from(
+            notFoundExistingInstanceIds
+          ).join(
+            ', '
+          )}. Nothing was changed. Call \`describe_instances\` to get valid ids (the \`id\` field of each instance), and check the scene and layer names.`
+        );
+      }
+
       instancesToDelete.forEach(instance => {
         initialInstances.removeInstance(instance);
       });
@@ -3862,6 +3910,20 @@ const put3dInstances: EditorFunction = {
       }
 
       if (notFoundExistingInstanceIds.size > 0) {
+        // If NONE of the requested instances were found and nothing new was
+        // created, the call did nothing. Return a failure so the agent gets a
+        // real error signal instead of a misleading success — a success here
+        // can make the agent retry the same (often malformed) call in a loop.
+        if (existingInstanceStates.size === 0 && newInstancesCount === 0) {
+          return makeGenericFailure(
+            `None of the specified instance ids were found: ${Array.from(
+              notFoundExistingInstanceIds
+            ).join(
+              ', '
+            )}. Nothing was changed. Call \`describe_instances\` to get valid ids (the \`id\` field of each instance), and check the scene and layer names.`
+          );
+        }
+
         changes.push(
           `Instance ids not found: ${Array.from(
             notFoundExistingInstanceIds