Speed up structural search: most-selective-first predicates + statement timeout

[skearnes]

Addresses the slow/hanging structural searches analyzed in the ord-logbook entry "Structural search performance in the ORD interface" (2026-06-26-structural-search-performance.md). Independent of the natural-language work (#203/#204); this touches the shared queries.py/search.py engine used by both /search and /ask.

1. Most-selective-first query shape (queries.py)

Previously each predicate produced a SELECT DISTINCT reaction_id FROM … branch, combined with INTERSECT, with LIMIT on top. Each branch was fully materialized (sort + unique) before the set operation, so LIMIT could not bound the per-branch index scans.

Now each ReactionQuery exposes where_predicate — an EXISTS (…) (or direct) condition correlated to a single outer ord.reaction row — and run_queries builds:

SELECT reaction.reaction_id FROM ord.reaction WHERE (p1) AND (p2) … LIMIT n

The planner can lead with the most selective predicate as a semi-join and stop once LIMIT rows are found. Measured against the live DB:

query	before	after
benzene EXACT input + yield > 70%	~25 s	~10 s
morphine SIMILAR product	~22 s	~1 s

(Similarity ranking is unchanged: candidates are collected, then scored/limited.)

2. statement_timeout floor (search.py)

get_cursor now sets statement_timeout (default 20 s, override ORD_INTERFACE_STATEMENT_TIMEOUT_MS) so genuinely pathological queries — a common-scaffold substructure with no selective co-filter, or a broad match with an empty result that can't early-terminate — are cancelled rather than running for 30–80 s. The resulting psycopg.errors.QueryCanceled maps to a 400 with an actionable "too broad — add constraints" message. The background-task path (run_task/fetch_query_result) records the error so a timed-out async search reports it instead of polling forever. Verified on the live DB: pyridine substructure now returns a 400 at ~20 s instead of hanging.

Tests

• All existing result-count tests (queries_test.py, search_test.py, sync + async) pass unchanged — the rewrite is result-preserving.
• Added unit tests for the QueryCanceled → 400 mapping and the background-task error path.

Notes

Fix #2 is the safety net; fix #1 is the real speedup where results exist. Remaining ideas for later (selectivity heuristics to reject lone broad substructure searches, SIMILAR threshold tuning, warm-cache/storage) are captured in the logbook entry.

🤖 Generated with Claude Code

Greptile Summary

This PR restructures the reaction search engine to eliminate expensive SELECT DISTINCT … INTERSECT query shapes in favour of a single SELECT … WHERE (EXISTS …) AND (EXISTS …) with an outer LIMIT, enabling the Postgres planner to lead with the most-selective predicate and stop early. A statement_timeout floor (default 20 s, env-overridable) is added to every DB connection, with QueryCanceled mapped to a 400 with an actionable user message; the background-task path (run_task/fetch_query_result) now stores the error in Redis so polling callers get a meaningful response instead of waiting indefinitely.

• queries.py: Abstract method renamed from query_and_parameters to where_predicate; each subclass now returns a correlated EXISTS (…) fragment; run_queries builds one FROM ord.reaction WHERE (p1) AND (p2) … LIMIT n query from the collected predicates.
• search.py: get_cursor sets statement_timeout; run_query catches psycopg.errors.QueryCanceled and raises a 400; run_task catches HTTPException and writes error:{task_id} to Redis; fetch_query_result checks that key before the result key.
• search_test.py: Two new async unit tests cover the QueryCanceled → 400 path and the background-task error-recording path using monkeypatching.

Confidence Score: 5/5

Safe to merge. The SQL rewrite is semantically equivalent to the original — all user values remain bound parameters, and the query shape change is well-reasoned and consistent across all seven query subclasses.

The SQL restructuring from INTERSECT-of-DISTINCT branches to a single correlated-EXISTS WHERE clause is correct across every subclass, existing result-count tests pass unchanged, the new timeout path is covered by two unit tests, and the error-propagation through the background-task pipeline is logically sound. No new defects were identified beyond the items already noted in prior review rounds.

No files require special attention. The _mols_source / _mols_join split in queries.py is worth a quick read if unfamiliar, but the logic is clearly commented and used correctly.

Important Files Changed

Filename	Overview
ord_interface/api/queries.py	Core query refactor: where_predicate replaces query_and_parameters across all seven subclasses; new _mols_source property provides a correlated FROM list for EXISTS subqueries while _mols_join is retained for the top-level similarity-scoring query. SQL assembly is safe — user values remain bound parameters.
ord_interface/api/search.py	Adds statement_timeout to every DB connection and correctly maps QueryCanceled to HTTP 400. Background-task error path is newly handled: run_task catches HTTPException, stores error:{task_id} in Redis, and returns early before writing the result key.
ord_interface/api/search_test.py	Two new async unit tests cover the QueryCanceled → 400 path and the background-task error-recording path using monkeypatching. Tests are well-scoped and deterministic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[HTTP GET /query or /submit_query] --> B[run_query]
    B --> C{build queries list}
    C --> D[get_cursor - statement_timeout=20s]
    D --> E[run_queries]
    E --> F{collect where_predicate per ReactionQuery}
    F --> G[SELECT reaction_id FROM ord.reaction WHERE p1 AND p2 LIMIT n]
    G -->|success| H{similarity ranking?}
    H -->|no| I[return reaction_ids]
    H -->|yes| J[_rank_by_similarity top-k by Tanimoto]
    J --> I
    G -->|QueryCanceled| K[raise HTTPException 400]
    B --> L{background task?}
    L -->|yes - run_task| M[try run_query]
    M -->|success| N[Redis SET result:task_id]
    M -->|HTTPException| O[Redis SET error:task_id]
    P[GET /fetch_query_result] --> Q{Redis EXISTS query:task_id}
    Q -->|no| R[404]
    Q -->|yes| S{error:task_id?}
    S -->|yes| T[return 400 + detail]
    S -->|no| U{result:task_id?}
    U -->|no| V[202 pending]
    U -->|yes| W[fetch_reactions - return results]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[HTTP GET /query or /submit_query] --> B[run_query]
    B --> C{build queries list}
    C --> D[get_cursor - statement_timeout=20s]
    D --> E[run_queries]
    E --> F{collect where_predicate per ReactionQuery}
    F --> G[SELECT reaction_id FROM ord.reaction WHERE p1 AND p2 LIMIT n]
    G -->|success| H{similarity ranking?}
    H -->|no| I[return reaction_ids]
    H -->|yes| J[_rank_by_similarity top-k by Tanimoto]
    J --> I
    G -->|QueryCanceled| K[raise HTTPException 400]
    B --> L{background task?}
    L -->|yes - run_task| M[try run_query]
    M -->|success| N[Redis SET result:task_id]
    M -->|HTTPException| O[Redis SET error:task_id]
    P[GET /fetch_query_result] --> Q{Redis EXISTS query:task_id}
    Q -->|no| R[404]
    Q -->|yes| S{error:task_id?}
    S -->|yes| T[return 400 + detail]
    S -->|no| U{result:task_id?}
    U -->|no| V[202 pending]
    U -->|yes| W[fetch_reactions - return results]

Loading

Comments Outside Diff (1)

1. ord_interface/api/search.py, line 82-104 (link)

   statement_timeout applies to all connections, not just search

   get_cursor() is called by several non-search endpoints — get_reaction, get_reactions, get_datasets, get_dataset, fetch_query_result (via fetch_reactions), input_stats, and product_stats — that do not wrap their usage in try/except psycopg.errors.QueryCanceled. If any of those queries somehow exceed 20 s (e.g., the input_stats aggregation under heavy DB load), they will surface as an unhandled QueryCanceled that FastAPI turns into a 500 Internal Server Error — not the graceful 400 with the user-friendly message. The PR description says "applied to every search connection" but the implementation is broader. In practice this is unlikely to matter, but a separate cursor factory (or a targeted try/except at each non-search site) would keep the intended scope explicit.

_{Reviews (2): Last reviewed commit: "Apply ruff format to search_test.py" | Re-trigger Greptile}

[greptile-apps]

Background task error handling is incomplete

run_task now correctly traps HTTPException (the timeout path) and records it in Redis so fetch_query_result can report it. However, other exceptions that can propagate from run_query — most notably a raw psycopg exception if the database connection drops mid-query — are not caught. In those cases neither result:{task_id} nor error:{task_id} is ever written, so callers polling fetch_query_result will receive 202 indefinitely until query:{task_id} expires after one hour. A broad except Exception fallback that writes a generic error:{task_id} entry would close this gap.