Skip to content

#5740 - Establishing connection between ambiguous phosphate and sugar works wrong#9681

Open
beyzaevcen wants to merge 1 commit intomasterfrom
5740-phosphate-sugar-connection
Open

#5740 - Establishing connection between ambiguous phosphate and sugar works wrong#9681
beyzaevcen wants to merge 1 commit intomasterfrom
5740-phosphate-sugar-connection

Conversation

@beyzaevcen
Copy link
Copy Markdown
Collaborator

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

(Screenshots, videos, or GIFs, if applicable)

Summary

  • Fixes a TypeError: this.getValidPoint is not a function crash when
    hovering a sugar over an ambiguous phosphate (or vice versa) in Macro
    Flex mode.
  • Fixes attachment points not being shown and the connection dialog
    incorrectly appearing when connecting an ambiguous phosphate/sugar
    with its counterpart.
  • The default ph(R2)–sg(R1) bond rule now applies automatically for
    variant monomers, per requirement §6.2.

Root cause

Two separate bugs:

1. TypeError in AmbiguousMonomer

AmbiguousMonomer.getValidSourcePoint delegates to the real class
prototype via .call(this, ...). For Phosphate and Sugar, those
prototype methods internally call a private helper getValidPoint.
Because this is an AmbiguousMonomer instance at call time,
getValidPoint is not in its prototype chain → TypeError.

2. Failed instanceof checks in Sugar and Phosphate

Sugar.getValidPoint checks instanceof Phosphate and
Phosphate.getValidPoint checks instanceof Sugar. Both return
false for AmbiguousMonomer instances, so ambiguous monomers were
never recognised as their equivalent type — causing the modal to open
instead of the bond being created automatically.

Changes

File Change
AmbiguousMonomer.ts Added _createContextProxy: a Proxy that intercepts property lookups and falls through to the target prototype for methods absent from AmbiguousMonomer (e.g. private getValidPoint)
monomers.ts Added isPhosphateOrAmbiguousPhosphate and isSugarOrAmbiguousSugar type guard helpers, following the existing isRnaBaseOrAmbiguousRnaBase / isPeptideOrAmbiguousPeptide pattern
Sugar.ts Replaced instanceof Phosphate with isPhosphateOrAmbiguousPhosphate
Phosphate.ts Replaced instanceof Sugar with isSugarOrAmbiguousSugar

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Establishing connection between ambiguous phosphate and sugar works wrong

1 participant