#9889 - Errors are displayed after switching between Expand and Colla…

packages/ketcher-core/__tests__/application/editor/actions/sgroup.test.ts

@@ -77,6 +77,40 @@ describe('setExpandMonomerSGroup', () => {
     (getAttachmentPointStereoBond as jest.Mock).mockReset();
   });
 
+  it('falls back to generic S-group collapse for monomers with >2 attachment points', () => {
+    const struct = new Struct();
+    const atom1Id = struct.atoms.add(
+      new Atom({ label: 'C', pp: new Vec2(0, 0) }),
+    );
+    const atom2Id = struct.atoms.add(
+      new Atom({ label: 'C', pp: new Vec2(1, 0) }),
+    );
+    const atom3Id = struct.atoms.add(
+      new Atom({ label: 'C', pp: new Vec2(2, 0) }),
+    );
+
+    const monomerSGroupId = createMonomerSGroup(struct, atom1Id);
+    struct.atomAddToSGroup(monomerSGroupId, atom2Id);
+    struct.atomAddToSGroup(monomerSGroupId, atom3Id);
+
+    addAttachmentPoint(struct, monomerSGroupId, atom1Id, 1);
+    addAttachmentPoint(struct, monomerSGroupId, atom2Id, 2);
+    addAttachmentPoint(struct, monomerSGroupId, atom3Id, 3);
+
+    const options = {
+      scale: 40,
+      width: 100,
+      height: 100,
+    } as unknown as RenderOptions;
+    const render = new Render(document as unknown as HTMLElement, options);
+    const restruct = new ReStruct(struct, render);
+
+    setExpandMonomerSGroup(restruct, monomerSGroupId, { expanded: false });
+
+    expect(struct.sgroups.get(monomerSGroupId)?.isExpanded()).toBe(false);
+    expect(getAttachmentPointStereoBond).not.toHaveBeenCalled();
+  });
+
   it('preserves stereo bonds when collapsing monomers', () => {
     const struct = new Struct();
     const atom1Id = struct.atoms.add(

packages/ketcher-core/src/application/editor/MacromoleculesConverter.ts

@@ -23,7 +23,7 @@ import {
 import type { BaseMonomer } from 'domain/entities/BaseMonomer';
 import { MonomerMicromolecule } from 'domain/entities/monomerMicromolecule';
 import { Command } from 'domain/entities/Command';
-import type { PolymerBond } from 'domain/entities/PolymerBond';
+import { PolymerBond } from 'domain/entities/PolymerBond';
 import assert from 'assert';
 import type { AttachmentPointName } from 'domain/types';
 import {
@@ -41,6 +41,133 @@ import { MONOMER_CONST } from 'domain/constants/monomers';
 import { MACROMOLECULES_BOND_TYPES } from 'application/editor/tools/types';
 
 export class MacromoleculesConverter {
+  private static findAttachmentPointByMonomerPair(
+    monomer: BaseMonomer,
+    polymerBond: PolymerBond,
+  ): AttachmentPointName | undefined {
+    const attachmentPointByBondId = Object.entries(
+      monomer.attachmentPointsToBonds,
+    ).find(([, monomerBond]) => {
+      return (
+        monomerBond instanceof PolymerBond && monomerBond.id === polymerBond.id
+      );
+    });
+
+    if (attachmentPointByBondId) {
+      const [attachmentPointName] = attachmentPointByBondId;
+
+      return attachmentPointName as AttachmentPointName;
+    }
+
+    const connectedMonomer = polymerBond.getAnotherMonomer(monomer);
+
+    if (!connectedMonomer) {
+      return undefined;
+    }
+
+    const matchingAttachmentPoints = Object.entries(
+      monomer.attachmentPointsToBonds,
+    ).filter(([, monomerBond]) => {
+      return (
+        monomerBond instanceof PolymerBond &&
+        monomerBond.getAnotherMonomer(monomer)?.id === connectedMonomer.id
+      );
+    });
+
+    if (matchingAttachmentPoints.length !== 1) {
+      return undefined;
+    }
+
+    const [attachmentPointName] = matchingAttachmentPoints[0];
+
+    return attachmentPointName as AttachmentPointName;
+  }
+
+  private static findAttachmentPointByBondId(
+    monomer: BaseMonomer,
+    bondId: number,
+  ): AttachmentPointName | undefined {
+    const matchingAttachmentPoint = Object.entries(
+      monomer.attachmentPointsToBonds,
+    ).find(([, monomerBond]) => monomerBond?.id === bondId);
+
+    if (!matchingAttachmentPoint) {
+      return undefined;
+    }
+
+    const [attachmentPointName] = matchingAttachmentPoint;
+
+    return attachmentPointName as AttachmentPointName;
+  }
+
+  private static syncAttachmentPointMappingForPolymerBonds(
+    drawingEntitiesManager: DrawingEntitiesManager,
+  ) {
+    drawingEntitiesManager.polymerBonds.forEach((polymerBond) => {
+      if (polymerBond instanceof HydrogenBond || !polymerBond.secondMonomer) {
+        return;
+      }
+
+      const firstAttachmentPoint =
+        polymerBond.firstMonomer.getAttachmentPointByBond(polymerBond);
+      if (!firstAttachmentPoint) {
+        const guessedFirstAttachmentPoint =
+          MacromoleculesConverter.findAttachmentPointByMonomerPair(
+            polymerBond.firstMonomer,
+            polymerBond,
+          );
+
+        if (guessedFirstAttachmentPoint) {
+          polymerBond.firstMonomer.setBond(
+            guessedFirstAttachmentPoint,
+            polymerBond,
+          );
+        }
+      }
+
+      const secondAttachmentPoint =
+        polymerBond.secondMonomer.getAttachmentPointByBond(polymerBond);
+      if (!secondAttachmentPoint) {
+        const guessedSecondAttachmentPoint =
+          MacromoleculesConverter.findAttachmentPointByMonomerPair(
+            polymerBond.secondMonomer,
+            polymerBond,
+          );
+
+        if (guessedSecondAttachmentPoint) {
+          polymerBond.secondMonomer.setBond(
+            guessedSecondAttachmentPoint,
+            polymerBond,
+          );
+        }
+      }
+    });
+  }
+
+  private static syncAttachmentPointMappingForMonomerToAtomBonds(
+    drawingEntitiesManager: DrawingEntitiesManager,
+  ) {
+    drawingEntitiesManager.monomerToAtomBonds.forEach((monomerToAtomBond) => {
+      const { monomer } = monomerToAtomBond;
+      const attachmentPoint =
+        monomer.getAttachmentPointByBond(monomerToAtomBond);
+
+      if (attachmentPoint) {
+        return;
+      }
+
+      const guessedAttachmentPoint =
+        MacromoleculesConverter.findAttachmentPointByBondId(
+          monomer,
+          monomerToAtomBond.id,
+        );
+
+      if (guessedAttachmentPoint) {
+        monomer.setBond(guessedAttachmentPoint, monomerToAtomBond);
+      }
+    });
+  }
+
   public static convertMonomerToMonomerMicromolecule(
     monomer: BaseMonomer,
     struct: Struct,
@@ -153,6 +280,13 @@ export class MacromoleculesConverter {
     struct: Struct,
     reStruct?: ReStruct,
   ) {
+    MacromoleculesConverter.syncAttachmentPointMappingForPolymerBonds(
+      drawingEntitiesManager,
+    );
+    MacromoleculesConverter.syncAttachmentPointMappingForMonomerToAtomBonds(
+      drawingEntitiesManager,
+    );
+
     const monomerToAtomIdMap = new Map<BaseMonomer, Map<number, number>>();
 
     drawingEntitiesManager.micromoleculesHiddenEntities.mergeInto(struct);

packages/ketcher-core/src/application/editor/actions/sgroup.ts

@@ -168,6 +168,13 @@ export function setExpandMonomerSGroup(
     return action;
   }
 
+  // Monomer-specific collapse adjustments are tuned for simple attachment
+  // topologies and can destabilize structures with >2 attachment points after
+  // mode switches. For collapse in such monomers, use generic S-Group logic.
+  if (!attrs.expanded && sGroup.getAttachmentPoints().length > 2) {
+    return setExpandSGroup(restruct, sgid, attrs);
+  }
+
   Object.keys(attrs).forEach((key) => {
     action.addOp(new SGroupAttr(sgid, key, attrs[key]));
   });

packages/ketcher-react/src/script/ui/views/components/ContextMenu/hooks/useFunctionalGroupEoc.test.ts

@@ -1,11 +1,36 @@
 import { renderHook } from '@testing-library/react';
 import useFunctionalGroupEoc from './useFunctionalGroupEoc';
-import type { FunctionalGroup } from 'ketcher-core';
+import {
+  Action,
+  ketcherProvider,
+  setExpandMonomerSGroup,
+  setExpandSGroup,
+  type FunctionalGroup,
+} from 'ketcher-core';
 import type {
   ItemEventParams,
   FunctionalGroupsContextMenuProps,
 } from '../contextMenu.types';
 
+jest.mock('ketcher-core', () => ({
+  Action: jest.fn().mockImplementation(() => ({
+    mergeWith: jest.fn(),
+  })),
+  ketcherProvider: {
+    getKetcher: jest.fn(() => ({
+      editor: {
+        render: { ctab: {} },
+        update: jest.fn(),
+        rotateController: {
+          rerender: jest.fn(),
+        },
+      },
+    })),
+  },
+  setExpandMonomerSGroup: jest.fn(() => ({})),
+  setExpandSGroup: jest.fn(() => ({})),
+}));
+
 jest.mock('react-redux', () => ({
   useDispatch: () => jest.fn(),
 }));
@@ -14,7 +39,94 @@ jest.mock('src/hooks', () => ({
   useAppContext: () => ({ ketcherId: 'test-ketcher-id' }),
 }));
 
+jest.mock('src/script/ui/state/functionalGroups', () => ({
+  highlightFG: jest.fn(),
+}));
+
 describe('useFunctionalGroupEoc', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe('handler function', () => {
+    it('uses setExpandSGroup for multi-selection collapse to avoid unstable monomer-specific path', () => {
+      const { result } = renderHook(() => useFunctionalGroupEoc());
+      const [handler] = result.current;
+
+      const functionalGroup1 = {
+        name: 'FG1',
+        isExpanded: true,
+        relatedSGroupId: 1,
+      } as FunctionalGroup;
+      const functionalGroup2 = {
+        name: 'FG2',
+        isExpanded: true,
+        relatedSGroupId: 2,
+      } as FunctionalGroup;
+
+      const params: ItemEventParams<FunctionalGroupsContextMenuProps> = {
+        props: {
+          id: 'test',
+          functionalGroups: [functionalGroup1, functionalGroup2],
+        },
+      } as ItemEventParams<FunctionalGroupsContextMenuProps>;
+
+      handler(params, false);
+
+      expect(ketcherProvider.getKetcher).toHaveBeenCalledWith(
+        'test-ketcher-id',
+      );
+      expect(setExpandSGroup).toHaveBeenCalledTimes(2);
+      expect(setExpandSGroup).toHaveBeenNthCalledWith(1, {}, 1, {
+        expanded: false,
+      });
+      expect(setExpandSGroup).toHaveBeenNthCalledWith(2, {}, 2, {
+        expanded: false,
+      });
+      expect(setExpandMonomerSGroup).not.toHaveBeenCalled();
+      expect(Action).toHaveBeenCalledTimes(1);
+
+      const editor = (ketcherProvider.getKetcher as jest.Mock).mock.results[0]
+        .value.editor;
+      expect(editor.update).toHaveBeenCalledTimes(1);
+      expect(editor.rotateController.rerender).toHaveBeenCalledTimes(1);
+    });
+
+    it('uses setExpandMonomerSGroup for multi-selection expand to preserve readable layout', () => {
+      const { result } = renderHook(() => useFunctionalGroupEoc());
+      const [handler] = result.current;
+
+      const functionalGroup1 = {
+        name: 'FG1',
+        isExpanded: false,
+        relatedSGroupId: 1,
+      } as FunctionalGroup;
+      const functionalGroup2 = {
+        name: 'FG2',
+        isExpanded: false,
+        relatedSGroupId: 2,
+      } as FunctionalGroup;
+
+      const params: ItemEventParams<FunctionalGroupsContextMenuProps> = {
+        props: {
+          id: 'test',
+          functionalGroups: [functionalGroup1, functionalGroup2],
+        },
+      } as ItemEventParams<FunctionalGroupsContextMenuProps>;
+
+      handler(params, true);
+
+      expect(setExpandMonomerSGroup).toHaveBeenCalledTimes(2);
+      expect(setExpandMonomerSGroup).toHaveBeenNthCalledWith(1, {}, 1, {
+        expanded: true,
+      });
+      expect(setExpandMonomerSGroup).toHaveBeenNthCalledWith(2, {}, 2, {
+        expanded: true,
+      });
+      expect(setExpandSGroup).not.toHaveBeenCalled();
+    });
+  });
+
   describe('hidden function', () => {
     it('should hide Contract Abbreviation when functional group has empty name', () => {
       const { result } = renderHook(() => useFunctionalGroupEoc());

packages/ketcher-react/src/script/ui/views/components/ContextMenu/hooks/useFunctionalGroupEoc.ts

@@ -1,4 +1,9 @@
-import { Action, ketcherProvider, setExpandMonomerSGroup } from 'ketcher-core';
+import {
+  Action,
+  ketcherProvider,
+  setExpandMonomerSGroup,
+  setExpandSGroup,
+} from 'ketcher-core';
 import { useCallback } from 'react';
 import { useDispatch } from 'react-redux';
 import { useAppContext } from 'src/hooks';
@@ -24,13 +29,23 @@ const useFunctionalGroupEoc = () => {
       const molecule = editor.render.ctab;
       const selectedFunctionalGroups = props?.functionalGroups;
       const action = new Action();
+      const isMultiSelection = (selectedFunctionalGroups?.length ?? 0) > 1;
+      const useGenericPath = isMultiSelection && !toExpand;
 
       selectedFunctionalGroups?.forEach((functionalGroup) => {
-        action.mergeWith(
-          setExpandMonomerSGroup(molecule, functionalGroup.relatedSGroupId, {
-            expanded: toExpand,
-          }),
-        );
+        if (useGenericPath) {
+          action.mergeWith(
+            setExpandSGroup(molecule, functionalGroup.relatedSGroupId, {
+              expanded: toExpand,
+            }),
+          );
+        } else {
+          action.mergeWith(
+            setExpandMonomerSGroup(molecule, functionalGroup.relatedSGroupId, {
+              expanded: toExpand,
+            }),
+          );
+        }
       });
 
       editor.update(action);