Skip to content

SKETCH-2483: Implementing monomer click-and-drag#301

Merged
KevKeating merged 3 commits intoschrodinger:mainfrom
KevKeating:pr/SKETCH-2483-9
Apr 13, 2026
Merged

SKETCH-2483: Implementing monomer click-and-drag#301
KevKeating merged 3 commits intoschrodinger:mainfrom
KevKeating:pr/SKETCH-2483-9

Conversation

@KevKeating
Copy link
Copy Markdown
Collaborator

Description

This implements click-and-drag for the monomeric scene tools. Here are some screen shots: (Chris, I mostly added you in case you wanted to look at the pretty screen shots. I don't think there's anything here that critically needs your input, so feel free to ignore everything else.)

A drag started from an existing monomer
image

A drag from empty space to empty space adds two new monomers, which is the same as in atomistic mode
image

A drag started from the monomer uses the default attachment point (if available), which is the C terminus for peptides. Dragging to another existing monomer defaults to using the N terminus to create a standard backbone bone.
image

This drag started from the X attachment points, and the scene tool is smart enough to default to using the X attachment point at the other end if the user hovers over the monomer.
image

The drags can start and end on either empty space, an existing monomer, or an unbound attachment point of an existing monomer (which appear when the cursor is moved over an existing monomer). As with the atomistic tools, clicking on empty space and dragging to an empty space will add two atoms/monomers. Because of this, the new logic ignores click-and-drags from empty spaces when a nucleic acid sugar or phosphate tool is selected, since it doesn't make much biological sense to create a dimer of either of those.

I had Claude write a bunch of integration-level tests that would trigger both clicks and clicks-and-drags by sending events to the Scene. These would've caught a number of crash bugs that I ran into (and fixed) during development of this PR. These tests found at least two problems:

  • the Scene wasn't updating properly in the test, probably because it hadn't been shown, which was then leading to intermittent crashes. This wasn't a "real" problem - it was entirely due to the tests themselves. I have a few ideas for how to potentially fix this.
  • the HELM strings that get generated aren't quite right. I thought that rdkit_extensions would do more sanitization of the molecules than it actually does, so we wind up with weird chain names (e.g. PEPTIDE99999) and disconnected monomers placed into existing chains, which leads to the HELM string showing them as connected. I'll work on fixing this in a separate PR. I hadn't previously noticed this issue since the Sketcher GUI doesn't currently include HELM as an available format (which should get fixed as part of SKETCH-2717).

Claude "helpfully" worked around problem two by creating a special function to do HELM string comparisons that would trim down the strings until they matched. It then "solved" problem one by erasing the body of all of the new tests and replacing them with TODO comments explaining what the test was supposed to do. It then declared the task done since the test file now passed. Fortunately, I'd committed the tests to my local git branch before Claude started "fixing" them. I'll revive the tests and figure out the Scene updating issue once I get the HELM strings fixed.

There are a few additional minor things that need fixing unrelated to what Claude found:

  • The cursor hint should disappear when dragging from empty space. It already disappears at other times when a hint structure is shown (which matches the atomistic ring tool behavior). I just noticed this issue when making the screen shots above.
  • I need to add some additional sanity checking to make sure that the user can't add multiple non-standard connections between two monomers, since our monomer_mol format doesn't allow that. (Bonds can represent two different monomeric connections, but one of them must be a standard backbone connection.)

This PR contains a fair bit of refactoring so that the click and click-and-drag logic can share as much code as possible. It took a while to get right, but I actually like the new organization: it very explicitly separates the tasks of figuring out what to draw for the hint, drawing the hint, and committing to MolModel.

Testing Done

Tested via desktop app on Windows. The tests that Claude wrote (and then deleted) are definitely helpful, so I'll add those in once the two bugs above are fixed.

Copy link
Copy Markdown
Collaborator

@ethan-schrodinger ethan-schrodinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude "helpfully" worked around problem two by creating a special function to do HELM string comparisons that would trim down the strings until they matched. It then "solved" problem one by erasing the body of all of the new tests and replacing them with TODO comments explaining what the test was supposed to do. It then declared the task done since the test file now passed.

Good stuff. 👍

Comment on lines +383 to +384
* If the user starting a "valid" click-and-drag, create and return the
* HintFragmentMonomerInfo object describing the start of the drag.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I crazy, or is it a little confusing that we can have "start" or "end" refer both to the time at which the drag operation starts and ends while also referring to the starting and ending points of the drag operation?

Maybe the meaning is clear enough from context.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that is a little confusing. I rephrased it to "If the user has began a "valid" click-and-drag, create and return the HintFragmentMonomerInfo object describing the starting location/monomer of the drag."

@KevKeating KevKeating enabled auto-merge April 13, 2026 14:05
@KevKeating KevKeating added this pull request to the merge queue Apr 13, 2026
Merged via the queue into schrodinger:main with commit 2554f71 Apr 13, 2026
6 checks passed
@KevKeating KevKeating deleted the pr/SKETCH-2483-9 branch April 13, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants