#8174-An IDT alias for one of the positions (5', internal, 3') must be unique in the whole library

[guliaisaeva]

Per the Indigo spec, an IDT alias for each position (5′, internal, 3′) must be unique across the whole monomer library. Previously, monomers/presets with colliding IDT modification aliases were silently loaded, leading to ambiguous IDT serialization. This PR validates IDT modification aliases during library load and user uploads, skips the offending monomer, and surfaces a clear error — while still loading the rest of the library.

How the feature works? / How did you fix the issue?

(Screenshots, videos, or GIFs, if applicable)

Changes
packages/ketcher-core/src/application/editor/Editor.ts

In CoreEditor.updateMonomersLibrary, added a validator that:
Collects endpoint5, internal, endpoint3 IDT aliases for each incoming monomer.
Rejects the monomer if the same alias is reused at multiple positions within itself (intra-monomer collision).
Rejects the monomer if any of its position aliases collides with a different position on an already-loaded monomer (cross-monomer, cross-position collision).
Error message includes the monomer name and the offending alias details, e.g.:
Editor::updateMonomersLibrary: IDT alias must be unique per position (5', internal, 3') across the whole library for monomer CHEM_CROSS_B (IDT base alias "IdtBaseB", IDT 5' alias "SharedMod"). The monomer was not added to the library.

Paired every existing KetcherLogger.error(...) in updateMonomersLibrary with a console.error(...) so validation failures are visible by default (without requiring window.ketcher.logging opt-in).

[claude]

Pairing console.error with KetcherLogger.error on every failure path (8 places in this PR) bypasses the logger abstraction — KetcherLogger.error intentionally gates output on settings.enabled/LogLevel, and the new lines defeat that gate and produce duplicate output when logging is already enabled.

If the goal is "validation failures must always be visible," consider one of:

• Update KetcherLogger.error (or add KetcherLogger.criticalError) so monomer-library validation failures always emit, instead of teaching every call site to bypass the logger.
• Surface the rejection through the UI/error channel rather than the console, since Ketcher is commonly embedded as an iframe where consumers can't suppress these console writes without monkey-patching console.

Same comment applies to the other 7 console.error additions in this file.

[claude]

Indigo#3161 is opaque for future readers — is it a GitHub issue, Jira ticket, internal spec? Consider replacing with a full URL (e.g. https://github.com/epam/Indigo/issues/3161) or inlining the one-sentence rationale, since the cross-repo shorthand will rot.

[claude]

This _monomersLibrary.some(...) overlaps with the same-position collision scan at lines 465–490: any cross-monomer modification-alias collision (including same-position ones) is now also caught here. That means lines 480–488 are effectively dead for the modification fields — but they fire first, so users will see two different error messages depending on whether the collision is same-position vs. cross-position, even though the underlying invariant is the same.

Two suggestions:

1. Consolidate the two passes. Build the set of all existing modification aliases once (alongside the existing HELM/BILN/base index) and check the new monomer against it in a single traversal. This also folds in the existing alias check for O(N+L) lookups instead of two O(N*L) scans — relevant once the library reaches the thousand-monomer range.

2. Make the error actionable. The current message (lines 534–538) lists the new monomer's own aliases but doesn't tell the user which alias collided or which existing monomer owns the conflicting value. The earlier Offending field(s): name="value" pattern (line 579) is a good model — capture { alias, otherMonomer } inside the .some() and include both in the message.

[claude]

"IDT alias must be unique per position (5', internal, 3') across the whole library" reads as "uniqueness is scoped to each position" — but the code actually enforces uniqueness across all three positions and across monomers (including intra-monomer between endpoint5/internal/endpoint3). Consider rewording to match what's enforced, e.g.:

"IDT modification alias (5'/internal/3') must be unique across all positions and all monomers in the library …"

This also matches the block comment above the check.

[claude]

Review summary — validation logic is correct and tests cover the new intra-/cross-position cases.

Main things worth addressing before merge:

• console.error + KetcherLogger.error pattern (8 sites). This bypasses the logger's enable/level gate and duplicates output when logging is on. If "always visible" is the requirement, update KetcherLogger rather than teaching every call site to skip it — Ketcher is often embedded, and consumers can't suppress raw console.error cleanly. (Flagged inline at line 441.)
• Error message for the new check is less actionable than nearby ones. It doesn't say which alias collided or with which monomer. Pattern after the Offending field(s): name="value" message a few lines down. (Inline at lines 519–528, 534.)
• Redundancy with the same-position collision scan above. Could be folded into one pass over _monomersLibrary (also helps as libraries scale toward thousands of monomers). (Inline at lines 519–528.)
• Minor: Indigo#3161 reference is opaque; misleading "unique per position" wording in the error.

Nothing security-blocking — alias values flow only to logs, not the DOM. Tests look reasonable; consider also asserting that a same-position cross-monomer collision still triggers the older "Alias collision detected" message so the two code paths stay distinguishable.