feat: look up compounds by name, SMILES, or InChI (#465) - #817
Open
skearnes wants to merge 1 commit into
Open
Conversation
"Look up name" previously hardcoded identifier_type=name, so pasting a SMILES or InChI only resolved when an external service happened to accept it as a name. - FE: add a Name/SMILES/InChI selector to the lookup dialog and thread the chosen type through addIdentifierByName to /resolve-compound. - BE: resolve_compound canonicalizes a SMILES locally (no remote lookup) and threads the type to the resolvers for name/InChI; also returns a clean 400 instead of a 500 when every resolver fails (the None-unpack bug). Tests: BE resolve_compound (SMILES short-circuit, type passthrough, 400 on miss); FE thunk passes the selected type through. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
🤖 Pulumi Neo didn't review this pull request: no Pulumi preview ran for it. Agentic reviews require a preview of the affected stacks (for example from your CI's |
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+27
to
+28
| // Resolver identifier type — 'name' | 'smiles' | 'inchi' (#465). | ||
| identifierType: string; |
There was a problem hiding this comment.
identifierType field is typed as string, which is looser than what the backend accepts and what the UI dropdown enforces. Narrowing it to a union literal lets the TypeScript compiler catch any mismatch at the call sites.
Suggested change
| // Resolver identifier type — 'name' | 'smiles' | 'inchi' (#465). | |
| identifierType: string; | |
| // Resolver identifier type (#465). | |
| identifierType: 'name' | 'smiles' | 'inchi'; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #465.
Problem
"Look up name" hardcoded
identifier_type: 'name', so pasting a SMILES or InChI only resolved when an external service happened to accept it as a free-text name — hence the inconsistent "works for some structures, not others."Changes
Frontend
addIdentifierByName→/resolve-compound.Backend (utilities.py)
name_resolve_cachedreturnsNone; the old code unpacked it (resolver, smiles = None) →TypeError. Now returns a clean 400.Tests
utilities_test.py): SMILES short-circuit (no remote call), identifier-type passthrough, and 400-on-unresolvable./resolve-compound.tsc -b, lint,ruff,tyall green.🤖 Generated with Claude Code
Greptile Summary
This PR fixes compound lookup so that SMILES and InChI identifiers are resolved correctly instead of always being treated as names. A new identifier-type dropdown is wired end-to-end from the UI through the Redux action and thunk to the
/resolve-compoundendpoint, which now short-circuits SMILES to a local RDKit canonicalization and also fixes a latent 500 that occurred when all remote resolvers returnedNone.Nonefrom exhausted resolvers now returns a 400 instead of aTypeError.ComponentsLookupgains a Name / SMILES / InChISelect; the chosen type travels through the action payload and thunk to the POST body.Confidence Score: 4/5
The core routing logic is correct and well-tested; the only gaps are a missing input constraint in the Pydantic schema and a loose TypeScript type that the frontend dropdown already enforces in practice.
The SMILES short-circuit, the None-guard fix, and the type passthrough all work correctly. The main weakness is that
ResolveCompoundInputs.identifier_typeis an unconstrainedstr, so an out-of-band caller sending an unrecognized type (e.g. wrong case) gets a generic "Could not resolve" error rather than a clear validation failure. The UI dropdown prevents this in the happy path, but aLiteralconstraint would close the gap. Everything else — the thunk change, action typing, and tests — is straightforward.ord_app/service_api/schemas/utilites.pydeserves a second look to add theLiteralconstraint onidentifier_type;ComponentsLookup.tsxhas a minor UX inaccuracy in the description text when SMILES is selected.Important Files Changed
Comments Outside Diff (1)
ui/src/features/reactions/ReactionEntities/entityFormConfiguration/components/CustomIdentifiers/ComponentsLookup/ComponentsLookup.tsx, line 101-122 (link)When the user picks "SMILES", the text "Searching in PubChem, CACTUS, eMolecules databases" remains visible even though the SMILES path never contacts those services — it is canonicalized locally. Consider hiding or replacing the databases sentence conditionally on
form.values.identifierType === 'smiles'.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "feat: look up compounds by name, SMILES,..." | Re-trigger Greptile