Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions ord_app/service_api/resources/v1/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
80 changes: 80 additions & 0 deletions ord_app/service_api/resources/v1/utilities_test.py
Original file line number Diff line number Diff line change
@@ -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):

Check warning on line 27 in ord_app/service_api/resources/v1/utilities_test.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use asynchronous features in this function or remove the `async` keyword.

See more on https://sonarcloud.io/project/issues?id=open-reaction-database_ord-app&issues=AZ7yYxFQ20gtn1HCeY8F&open=AZ7yYxFQ20gtn1HCeY8F&pullRequest=817
remote_calls.append((value_type, identifier))
return ("PubChem API", "unused")

Check failure on line 29 in ord_app/service_api/resources/v1/utilities_test.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "PubChem API" 3 times.

See more on https://sonarcloud.io/project/issues?id=open-reaction-database_ord-app&issues=AZ7yYxFQ20gtn1HCeY8D&open=AZ7yYxFQ20gtn1HCeY8D&pullRequest=817

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",

Check failure on line 37 in ord_app/service_api/resources/v1/utilities_test.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "/api/v1/resolve-compound" 3 times.

See more on https://sonarcloud.io/project/issues?id=open-reaction-database_ord-app&issues=AZ7yYxFQ20gtn1HCeY8E&open=AZ7yYxFQ20gtn1HCeY8E&pullRequest=817
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):

Check warning on line 51 in ord_app/service_api/resources/v1/utilities_test.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use asynchronous features in this function or remove the `async` keyword.

See more on https://sonarcloud.io/project/issues?id=open-reaction-database_ord-app&issues=AZ7yYxFQ20gtn1HCeY8G&open=AZ7yYxFQ20gtn1HCeY8G&pullRequest=817
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):

Check warning on line 71 in ord_app/service_api/resources/v1/utilities_test.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use asynchronous features in this function or remove the `async` keyword.

See more on https://sonarcloud.io/project/issues?id=open-reaction-database_ord-app&issues=AZ7yYxFQ20gtn1HCeY8H&open=AZ7yYxFQ20gtn1HCeY8H&pullRequest=817
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
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<ComponentsLookupProps>) {
const dispatch = useAppDispatch();
const { reactionId, pathComponents } = useContext(reactionEntityContext);
Expand All @@ -43,6 +50,7 @@ export function ComponentsLookup({ onClose }: Readonly<ComponentsLookupProps>) {
clearInputErrorOnChange: true,
initialValues: {
search: '',
identifierType: 'name',
},
});

Expand All @@ -60,14 +68,18 @@ export function ComponentsLookup({ onClose }: Readonly<ComponentsLookupProps>) {
}
}, [dispatch, hasError, values.search]);

const onSubmit = (values: { search: string }, event?: FormEvent<HTMLFormElement>) => {
const onSubmit = (
values: { search: string; identifierType: string },
event?: FormEvent<HTMLFormElement>,
) => {
event?.stopPropagation();
const path = pathComponents.concat('identifiers');
dispatch(
addIdentifierByName({
reactionId: reactionId,
pathComponents: path,
name: values.search,
identifierType: values.identifierType,
}),
);
};
Expand Down Expand Up @@ -108,8 +120,15 @@ export function ComponentsLookup({ onClose }: Readonly<ComponentsLookupProps>) {
</Anchor>
databases
</Text>
<Select
label="Identifier type"
data={IDENTIFIER_TYPE_OPTIONS}
allowDeselect={false}
{...form.getInputProps('identifierType')}
/>
<TextInput
label="Compound name"
label="Compound identifier"
placeholder="Name, SMILES, or InChI"
{...inputProps}
error={hasError ? 'Compound not found' : inputProps.error}
data-autofocus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ import type { ReactionId } from 'store/entities/reactions/reactions.types.ts';
const { createAsyncAction } = createActionFactory('reactionInputs');

export const addIdentifierByNameActions = createAsyncAction<
{ reactionId: ReactionId; pathComponents: ReactionPathComponents; name: string },
{
reactionId: ReactionId;
pathComponents: ReactionPathComponents;
name: string;
// Resolver identifier type — 'name' | 'smiles' | 'inchi' (#465).
identifierType: string;
Comment on lines +27 to +28

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The 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!

},
void,
void
>('add_identifier_by_name');
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('addIdentifierByName', () => {
reactionId: 99,
pathComponents,
name: 'water',
identifierType: 'name',
}) as unknown as UnknownAction,
);

Expand All @@ -78,6 +79,23 @@ describe('addIdentifierByName', () => {
expect(arg.newValue.details).toBe('water');
});

it('passes the chosen identifier type through to the resolver (#465)', async () => {
const { store } = makeStore();
await store.dispatch(
addIdentifierByName({
reactionId: 99,
pathComponents,
name: 'InChI=1S/H2O/h1H2',
identifierType: 'inchi',
}) as unknown as UnknownAction,
);

expect(axiosMock.post).toHaveBeenCalledWith('/resolve-compound', {
identifier_type: 'inchi',
identifier: 'InChI=1S/H2O/h1H2',
});
});

it('dispatches failure and does not update the reaction when resolution rejects', async () => {
axiosMock.post.mockRejectedValueOnce(new Error('not found'));
const { store, types } = makeStore();
Expand All @@ -86,6 +104,7 @@ describe('addIdentifierByName', () => {
reactionId: 99,
pathComponents,
name: 'bogus',
identifierType: 'name',
}) as unknown as UnknownAction,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ import { ordCompoundIdentifierToReaction } from 'store/entities/reactions/reacti

export const addIdentifierByName = createThunkWithExplicitResult(
addIdentifierByNameActions,
({ reactionId, pathComponents, name }) =>
({ reactionId, pathComponents, name, identifierType }) =>
async (dispatch, getState) => {
try {
const result = await axiosInstance.post<{ smiles: string }>(
'/resolve-compound',
{
identifier_type: 'name',
identifier_type: identifierType,
identifier: name,
},
);
Expand Down
Loading