From 6304961a42f3ab00223b8d7de7a419ed458cbe7b Mon Sep 17 00:00:00 2001 From: Steven Kearnes Date: Mon, 22 Jun 2026 22:49:11 -0400 Subject: [PATCH] feat: look up compounds by name, SMILES, or InChI (#465) "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) --- ord_app/service_api/resources/v1/utilities.py | 22 ++++- .../resources/v1/utilities_test.py | 80 +++++++++++++++++++ .../ComponentsLookup/ComponentsLookup.tsx | 25 +++++- .../reactionsInputs/reactionInputs.actions.ts | 8 +- .../reactionInputs.thunks.test.ts | 19 +++++ .../reactionsInputs/reactionInputs.thunks.ts | 4 +- 6 files changed, 148 insertions(+), 10 deletions(-) create mode 100644 ord_app/service_api/resources/v1/utilities_test.py diff --git a/ord_app/service_api/resources/v1/utilities.py b/ord_app/service_api/resources/v1/utilities.py index 07bb836b..c9d5628f 100644 --- a/ord_app/service_api/resources/v1/utilities.py +++ b/ord_app/service_api/resources/v1/utilities.py @@ -68,11 +68,25 @@ async def resolve_input(input_string: str) -> str | Response: @router.post("/resolve-compound", response_model=ResolveCompoundOutputs) async def resolve_compound(inputs: ResolveCompoundInputs) -> dict | Response: - """Resolves a compound identifier into a SMILES string.""" + """Resolves a compound identifier (name/SMILES/InChI) into a canonical SMILES string. + + A SMILES is already a structure, so it is canonicalized locally without a remote lookup; + other identifier types (name, InChI) are resolved via the external services, with the type + threaded through so e.g. PubChem searches by InChI rather than treating it as a name. (#465) + """ try: - resolver, smiles = await name_resolve_cached( - inputs.identifier_type, inputs.identifier - ) + if inputs.identifier_type == "smiles": + return { + "smiles": canonicalize_smiles_cached(inputs.identifier), + "resolver": "RDKit", + } + result = await name_resolve_cached(inputs.identifier_type, inputs.identifier) + if result is None: + # Every resolver failed/returned nothing -- a clean 400, not a 500 from unpacking None. + return Response( + "Could not resolve the compound identifier.", status_code=400 + ) + resolver, smiles = result return {"smiles": canonicalize_smiles_cached(smiles), "resolver": resolver} except ValueError as error: return Response(str(error), status_code=400) diff --git a/ord_app/service_api/resources/v1/utilities_test.py b/ord_app/service_api/resources/v1/utilities_test.py new file mode 100644 index 00000000..fe0d38cf --- /dev/null +++ b/ord_app/service_api/resources/v1/utilities_test.py @@ -0,0 +1,80 @@ +# Copyright 2026 Open Reaction Database Project Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for the compound-resolution endpoint, focused on identifier-type routing (#465).""" + +from fastapi import status + +from ord_app.service_api.resources.v1 import utilities + + +def test_resolve_compound_smiles_canonicalizes_locally( + api_client, mock_authenticated_user, monkeypatch +): + # A SMILES is already a structure: canonicalize locally, never hit the remote resolvers. + remote_calls = [] + + async def fake_remote(value_type, identifier): + remote_calls.append((value_type, identifier)) + return ("PubChem API", "unused") + + monkeypatch.setattr(utilities, "name_resolve_cached", fake_remote) + monkeypatch.setattr( + utilities, "canonicalize_smiles_cached", lambda smiles: f"canon:{smiles}" + ) + + response = api_client.post( + "/api/v1/resolve-compound", + json={"identifier_type": "smiles", "identifier": "C(C)O"}, + ).raise_for_status() + + assert response.json() == {"smiles": "canon:C(C)O", "resolver": "RDKit"} + assert remote_calls == [] # no remote lookup for a SMILES + + +def test_resolve_compound_threads_identifier_type( + api_client, mock_authenticated_user, monkeypatch +): + # Non-SMILES types (name/InChI) go to the remote resolvers with the type passed through. + remote_calls = [] + + async def fake_remote(value_type, identifier): + remote_calls.append((value_type, identifier)) + return ("PubChem API", "O") + + monkeypatch.setattr(utilities, "name_resolve_cached", fake_remote) + monkeypatch.setattr(utilities, "canonicalize_smiles_cached", lambda smiles: smiles) + + response = api_client.post( + "/api/v1/resolve-compound", + json={"identifier_type": "inchi", "identifier": "InChI=1S/H2O/h1H2"}, + ).raise_for_status() + + assert response.json() == {"smiles": "O", "resolver": "PubChem API"} + assert remote_calls == [("inchi", "InChI=1S/H2O/h1H2")] + + +def test_resolve_compound_unresolvable_returns_400( + api_client, mock_authenticated_user, monkeypatch +): + # Every resolver failing yields None; surface a clean 400 rather than a 500 from unpacking None. + async def fake_remote(value_type, identifier): + return None + + monkeypatch.setattr(utilities, "name_resolve_cached", fake_remote) + + response = api_client.post( + "/api/v1/resolve-compound", + json={"identifier_type": "name", "identifier": "nonexistent-compound"}, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/ui/src/features/reactions/ReactionEntities/entityFormConfiguration/components/CustomIdentifiers/ComponentsLookup/ComponentsLookup.tsx b/ui/src/features/reactions/ReactionEntities/entityFormConfiguration/components/CustomIdentifiers/ComponentsLookup/ComponentsLookup.tsx index 51a9f3ff..a194e672 100644 --- a/ui/src/features/reactions/ReactionEntities/entityFormConfiguration/components/CustomIdentifiers/ComponentsLookup/ComponentsLookup.tsx +++ b/ui/src/features/reactions/ReactionEntities/entityFormConfiguration/components/CustomIdentifiers/ComponentsLookup/ComponentsLookup.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ import { Form, useForm } from '@mantine/form'; -import { Anchor, Button, Flex, Modal, Text, TextInput } from '@mantine/core'; +import { Anchor, Button, Flex, Modal, Select, Text, TextInput } from '@mantine/core'; import { useAppDispatch } from 'store/useAppDispatch.ts'; import { addIdentifierByName } from 'store/entities/reactions/reactionsInputs/reactionInputs.thunks.ts'; import { useContext, useEffect, type FormEvent } from 'react'; @@ -32,6 +32,13 @@ interface ComponentsLookupProps { onClose: () => void; } +// Resolver identifier types. Values match what the backend / PubChem expect (#465). +const IDENTIFIER_TYPE_OPTIONS = [ + { value: 'name', label: 'Name' }, + { value: 'smiles', label: 'SMILES' }, + { value: 'inchi', label: 'InChI' }, +]; + export function ComponentsLookup({ onClose }: Readonly) { const dispatch = useAppDispatch(); const { reactionId, pathComponents } = useContext(reactionEntityContext); @@ -43,6 +50,7 @@ export function ComponentsLookup({ onClose }: Readonly) { clearInputErrorOnChange: true, initialValues: { search: '', + identifierType: 'name', }, }); @@ -60,7 +68,10 @@ export function ComponentsLookup({ onClose }: Readonly) { } }, [dispatch, hasError, values.search]); - const onSubmit = (values: { search: string }, event?: FormEvent) => { + const onSubmit = ( + values: { search: string; identifierType: string }, + event?: FormEvent, + ) => { event?.stopPropagation(); const path = pathComponents.concat('identifiers'); dispatch( @@ -68,6 +79,7 @@ export function ComponentsLookup({ onClose }: Readonly) { reactionId: reactionId, pathComponents: path, name: values.search, + identifierType: values.identifierType, }), ); }; @@ -108,8 +120,15 @@ export function ComponentsLookup({ onClose }: Readonly) { databases +