8188 monomer library error reporting

[Bidzinashvili]

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

This change makes monomer library loading fail loudly instead of silently when SDF input is malformed.
If ketcher.updateMonomersLibrary() receives non-KET data, Ketcher now converts it to KET inside ensureMonomersLibraryDataInKetFormat(). If the conversion fails, the error is wrapped with a clearer message:
Monomer item could not be loaded because of an error: ...
That error now propagates to the caller and is also logged to the console, so broken monomer presets are no longer ignored without feedback.
I also added a console catch in macromolecules editor initialization so async monomer library loading failures are not lost at startup.
Added an e2e test for the broken SDF case from the issue to verify the error is thrown and the preset is not added to the library.

Check list

• unit-tests written
• e2e-tests written
• documentation updated
• PR name follows the pattern #1234 – issue name
• branch name doesn't contain #
• PR is linked with the issue
• base branch (master or release/xx) is correct
• task status changed to "Code review"
• reviewers are notified about the pull request

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR makes monomer library loading failures visible by converting non-KET monomer data to KET with explicit error wrapping, surfacing validation errors, and adding startup logging so async failures aren’t silently ignored.

Changes:

• Wrap struct-service conversion failures with a clearer monomer-load error message.
• Collect monomer library validation errors and throw them after attempting to update the library.
• Update macromolecules editor init to log async monomer library init failures; enable previously test.fail’d e2e cases.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
packages/ketcher-macromolecules/src/state/common/editorSlice.ts	Logs rejected monomer-library init Promise during editor startup.
packages/ketcher-core/src/application/ketcher.ts	Wrapes conversion errors into a clearer monomer-load error.
packages/ketcher-core/src/application/editor/Editor.ts	Aggregates validation errors, logs them, and throws after update dispatch.
ketcher-autotests/tests/specs/Chromium-popup/Library/library-update.spec.ts	Enables e2e cases previously marked as expected failures.

[claude]

Review Summary

Overall the partial-success aggregation pattern with a custom MonomerLibraryUpdateError is a good direction. A few items raised inline; flagging two more here that I couldn't anchor to a specific diff line:

1. updateMonomersLibrary throws after this.events.updateMonomersLibrary.dispatch() (Editor.ts:648–651)

Valid monomers are already committed to _monomersLibrary and the UI has been refreshed by the time the caller sees the error. This is reasonable partial-success semantics, but the contract is undocumented and untested:

• No JSDoc on updateMonomersLibrary or MonomerLibraryUpdateError declares this behavior.
• No unit test explicitly asserts "given mixed valid+invalid input, the function throws AND the valid items are present in monomersLibrary". The mixed-aliases test at Editor.test.ts:247-315 covers this incidentally but doesn't frame it as a contract.

Worth adding both, since callers who treat the throw as a rollback will be surprised.

2. Inconsistent error collection inside updateMonomersLibrary (Editor.ts)

The new reportValidationError helper (L401) is a clean centralization, but several rejection paths in the same function still call KetcherLogger.error(...) directly and do not contribute to the thrown MonomerLibraryUpdateError:

• L463 — invalid BILN alias
• L544 — IDT alias too long
• L580 — IDT alias collision (existing)
• L628 — monomer group template name too long

updateMonomersLibrary returns normally for those items. That contradicts the PR's "fail loudly" goal and produces an inconsistent contract — same function, same kind of rejection, two different observable behaviors. Either route all rejection paths through reportValidationError, or comment why some are deliberately silent.

3. Public API surface considerations (docs)

MonomerLibraryUpdateError is re-exported via application/editor/index.ts → src/index.ts (export *), so it's part of the public ketcher-core API. Worth:

• Adding a TSDoc block on the class explaining when it's thrown and what partialSuccess / skippedItems mean.
• Adding a **Throws:** callout to the updateMonomersLibrary / replaceMonomersLibrary sections in README.md (around lines 1035 and 1077). The previous contract was "always resolves"; this is a behavior change worth documenting for SDK consumers.

Performance / security

Independent passes for performance and security found nothing material.

Positives

• Custom error subclass (name set, extends Error) is the right pattern.
• Using { cause } in ketcher.ts:789 preserves the original error in the chain.
• Autotest changes flipping test.fail → test with toContain(...) assertions are tightly scoped and verify the new behavior end-to-end.

[claude]

Code review summary

The PR achieves its stated goal for most validators — MonomerLibraryUpdateError propagates loudly, the e2e suite now drops test.fail markers, and unit tests assert the throw. A few items worth addressing before merge (specifics in inline threads):

Inconsistency: not all validators were converted

The new reportValidationError(...) helper is used for 6 of the 9 validation sites, but three sibling paths in packages/ketcher-core/src/application/editor/Editor.ts still call KetcherLogger.error(errorMessage) directly and never accumulate into validationErrors:

• line 463 — invalid BILN alias
• line 544 — IDT alias exceeds max length (IDT_ALIAS_LENGTH_ERROR_MESSAGE)
• line 629 — monomer-group-template name exceeds max length (MONOMER_GROUP_TEMPLATE_NAME_MAX_LENGTH_ERROR_MESSAGE)

These cases remain silent failures, which contradicts the PR's stated goal of failing loudly on malformed input. Either route them through reportValidationError, or add a comment explaining why they're intentionally excluded. (Inline anchor wasn't possible — these lines fall between diff hunks.)

Other inline notes

• MonomerLibraryUpdateError.partialSuccess is hardcoded true even when every input item failed — see inline at Editor.ts:140.
• editorSlice.ts uses bare console.error rather than KetcherLogger — see inline.
• Public contract change (now throws) lacks JSDoc / README update — see inline on ketcher.ts.

Non-blocking

• Tests assert on prose substrings (expect(error).toContain('invalid HELM alias value')). Stable error codes on MonomerLibraryUpdateError entries would let tests assert kind without coupling to copy, but that's a future refactor.
• Security review: no exploitable XSS — all current sinks for these messages are React-escaped. Worth a brief code comment on MonomerLibraryUpdateError warning that messages contain untrusted input and must not be rendered via dangerouslySetInnerHTML.