Deduplicate stats queries and return proper HTTP errors

Backend consistency cleanup (Tier 2), no change to successful responses:

- Collapse the near-identical `fetch_dataset_most_used_smiles_for_inputs`
  and `..._for_products` into one private helper, with the input/product
  table names passed as safely-composed `psycopg.sql.Identifier`s rather
  than duplicated query text.
- Replace bare `ValueError`s in API handlers with `HTTPException`s so
  clients get meaningful status codes instead of 500s:
  - `/molfile` with an unparseable SMILES -> 400
  - `/reaction_summary` for an unknown reaction_id -> 404 (and 500 only
    for the genuinely-impossible "multiple matches" case)
- Add regression tests for both error paths.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: skearnes

ord_interface/api/queries.py

@@ -50,7 +50,7 @@
 from ord_schema import message_helpers, validations
 from ord_schema.logging import get_logger
 from ord_schema.proto import reaction_pb2
-from psycopg import AsyncCursor
+from psycopg import AsyncCursor, sql
 from pydantic import BaseModel
 from rdkit import Chem
 from rdkit.Chem import rdChemReactions
@@ -435,47 +435,76 @@ class StatsResult(BaseModel):
     times_appearing: int
 
 
-async def fetch_dataset_most_used_smiles_for_inputs(
-    cursor: DictCursor, dataset_id: str, limit: int = 30
+async def _fetch_dataset_most_used_smiles(
+    cursor: DictCursor,
+    dataset_id: str,
+    *,
+    compound_table: str,
+    join_table: str,
+    foreign_key: str,
+    limit: int,
 ) -> list[StatsResult]:
-    """Fetches the top K most used SMILES molecules in terms of reaction inputs for a given dataset."""
-    query = """
-            SELECT smiles, COUNT(*) as times_appearing
-            FROM ord.compound
-            JOIN ord.reaction_input ON ord.compound.reaction_input_id = ord.reaction_input.id
-            JOIN ord.reaction ON ord.reaction_input.reaction_id = ord.reaction.id
+    """Fetches the top K most used SMILES for a dataset, joined through the given tables.
+
+    Args:
+        cursor: Database cursor.
+        dataset_id: Dataset to aggregate over.
+        compound_table: Table holding SMILES, e.g. "compound" or "product_compound".
+        join_table: Table linking compounds to reactions, e.g. "reaction_input".
+        foreign_key: Column on ``compound_table`` referencing ``join_table``.
+        limit: Maximum number of rows to return.
+
+    Returns:
+        Most frequently appearing SMILES, in descending order of frequency.
+    """
+    # Compose schema-qualified identifiers safely rather than interpolating raw
+    # strings into the SQL text.
+    compound = sql.Identifier("ord", compound_table)
+    join = sql.Identifier("ord", join_table)
+    query = sql.SQL(
+        """
+            SELECT smiles, COUNT(*) AS times_appearing
+            FROM {compound}
+            JOIN {join} ON {compound}.{foreign_key} = {join}.id
+            JOIN ord.reaction ON {join}.reaction_id = ord.reaction.id
             JOIN ord.dataset ON ord.reaction.dataset_id = ord.dataset.id
             WHERE ord.dataset.dataset_id = %s
             AND smiles IS NOT NULL
             GROUP BY smiles
             ORDER BY times_appearing DESC
             LIMIT %s
         """
+    ).format(compound=compound, join=join, foreign_key=sql.Identifier(foreign_key))
     await cursor.execute(query, (dataset_id, limit))
     results = []
     async for row in cursor:
         results.append(StatsResult(**row))
     return results
 
 
+async def fetch_dataset_most_used_smiles_for_inputs(
+    cursor: DictCursor, dataset_id: str, limit: int = 30
+) -> list[StatsResult]:
+    """Fetches the top K most used SMILES molecules in terms of reaction inputs for a given dataset."""
+    return await _fetch_dataset_most_used_smiles(
+        cursor,
+        dataset_id,
+        compound_table="compound",
+        join_table="reaction_input",
+        foreign_key="reaction_input_id",
+        limit=limit,
+    )
+
+
 async def fetch_dataset_most_used_smiles_for_products(
     cursor: DictCursor, dataset_id: str, limit: int = 30
 ) -> list[StatsResult]:
     """Fetches the top K most used SMILES molecules in terms of reaction products for a given dataset."""
-    query = """
-            SELECT smiles, COUNT(*) as times_appearing
-            FROM ord.product_compound
-            JOIN ord.reaction_outcome ON ord.product_compound.reaction_outcome_id = ord.reaction_outcome.id
-            JOIN ord.reaction ON ord.reaction_outcome.reaction_id = ord.reaction.id
-            JOIN ord.dataset ON ord.reaction.dataset_id = ord.dataset.id
-            WHERE ord.dataset.dataset_id = %s
-            AND smiles IS NOT NULL
-            GROUP BY smiles
-            ORDER BY times_appearing DESC
-            LIMIT %s
-        """
-    await cursor.execute(query, (dataset_id, limit))
-    results = []
-    async for row in cursor:
-        results.append(StatsResult(**row))
-    return results
+    return await _fetch_dataset_most_used_smiles(
+        cursor,
+        dataset_id,
+        compound_table="product_compound",
+        join_table="reaction_outcome",
+        foreign_key="reaction_outcome_id",
+        limit=limit,
+    )

ord_interface/api/search.py

@@ -224,7 +224,7 @@ async def get_molfile(smiles: str) -> str:
     """Returns a molblock for the given SMILES."""
     mol = Chem.MolFromSmiles(smiles)
     if mol is None:
-        raise ValueError(smiles)
+        raise HTTPException(status_code=400, detail=f"invalid SMILES: {smiles}")
     return Chem.MolToMolBlock(mol)
 
 

ord_interface/api/search_test.py

@@ -98,6 +98,11 @@ def test_get_molfile(test_client):
     assert Chem.MolToSmiles(Chem.MolFromMolBlock(response.json())) == "NC=O"
 
 
+def test_get_molfile_invalid_smiles(test_client):
+    response = test_client.get("/api/molfile", params={"smiles": "not-a-smiles"})
+    assert response.status_code == 400
+
+
 def test_get_search_results(test_client):
     response = test_client.post(
         "/api/download_search_results", json={"reaction_ids": ["ord-3f67aa5592fd434d97a577988d3fd241"]}

ord_interface/api/view.py

@@ -14,7 +14,7 @@
 
 """View API."""
 
-from fastapi import APIRouter, Request
+from fastapi import APIRouter, HTTPException, Request
 from fastapi.responses import HTMLResponse
 from ord_schema.proto import reaction_pb2
 
@@ -39,8 +39,10 @@ async def get_compound(request: Request) -> str:
 async def get_reaction_summary(reaction_id: str, compact: bool = True) -> str:
     """Renders a reaction as an HTML table with images and text."""
     results = await get_reactions(ReactionIdList(reaction_ids=[reaction_id]))
-    if len(results) == 0 or len(results) > 1:
-        raise ValueError(reaction_id)
+    if len(results) == 0:
+        raise HTTPException(status_code=404, detail=f"reaction not found: {reaction_id}")
+    if len(results) > 1:
+        raise HTTPException(status_code=500, detail=f"multiple reactions found: {reaction_id}")
     try:
         return generate_text.generate_html(reaction=results[0].reaction, compact=compact)
     except (ValueError, KeyError):

ord_interface/api/view_test.py

@@ -27,3 +27,8 @@ def test_get_compound_svg(test_client):
 def test_get_reaction_summary(test_client):
     response = test_client.get("/api/reaction_summary", params={"reaction_id": "ord-3f67aa5592fd434d97a577988d3fd241"})
     response.raise_for_status()
+
+
+def test_get_reaction_summary_not_found(test_client):
+    response = test_client.get("/api/reaction_summary", params={"reaction_id": "ord-does-not-exist"})
+    assert response.status_code == 404