diff --git a/CLAUDE.md b/CLAUDE.md index ec5676a8..45657a39 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,15 +59,17 @@ pip install -r requirements.txt 4. Results are scored, normalized, and returned as JSON ### Key Files -- `api/server.py` - Core FastAPI application (~717 lines): all endpoints, Pydantic models, Solr query construction, environment config +- `api/server.py` - Core FastAPI application: all endpoints, Pydantic models, Solr query construction, environment config - `api/apidocs.py` - Custom OpenAPI schema construction -- `api/resources/.openapi.yml` - OpenAPI 3.0.2 spec with service metadata +- `api/resources/openapi.yml` - OpenAPI 3.0.2 spec with service metadata - `main.py` / `main.sh` - WSGI/ASGI entry points (port 2433) - `tests/test_service.py` - Integration tests using FastAPI `TestClient` +- `tests/test_exact_mode.py` - Integration tests for the `exact` parameter - `tests/data/test-synonyms.json` - Test dataset for Solr ### Environment Variables - `SOLR_HOST` / `SOLR_PORT` - Solr connection (default: `localhost:8983`) +- `SOLR_MAX_CONCURRENT_LOOKUPS` / `SOLR_TIMEOUT_SECONDS` - Bulk-lookup fan-out bound and Solr query timeout (see `documentation/Deployment.md`) - `LOGLEVEL` - Logging level - `SERVER_ROOT` - API root path prefix - `MATURITY_VALUE` / `LOCATION_VALUE` - TRAPI metadata fields @@ -89,6 +91,12 @@ Solr documents contain: `curie`, `preferred_name`, `names` (synonym list), and b - **Data loading** - Separate pipeline in `data-loading/` (Makefile-driven, also has Kubernetes configs) - **CI/CD** - GitHub Actions: runs tests on push, publishes Docker image to GitHub Packages on release +## Gotchas + +- **The default search is *tokenized*, not *fuzzy*.** It matches the query's tokens in any order (order and adjacency are rewarded by the phrase-field boost, not required), and `autocomplete=true` makes the final token a prefix. There is no edit-distance matching, and `lookup()` escapes Solr's `~` out of the query so callers cannot request it. Do not describe it as "fuzzy" in docs or parameter descriptions -- that promises typo tolerance the service has never had. `tests/test_service.py` pins both halves of this. +- **Solr's `filterCache` is bounded by entry count (512), not by memory.** It earns its keep on shared, reusable filters (`types:`, `taxa:`, `curie:`). A filter whose value varies per query -- as exact mode's does -- must be marked `{!cache=false}`, or one bulk request evicts the whole cache and slows the ordinary search path down as collateral damage. See the comment in `lookup()`. +- **Query-side string normalization must not be applied to exact matching.** The `*_exactish` fields are a KeywordTokenizer plus a LowerCaseFilter and fold nothing else, so the smart-quote rewrite (and anything like it) would search for a string the caller never typed. The default path is unaffected because StandardTokenizer discards the punctuation anyway. + ## Documentation - `documentation/API.md` - Endpoint reference - `documentation/Deployment.md` - Docker/Kubernetes deployment guide diff --git a/api/resources/openapi.yml b/api/resources/openapi.yml index b06f18fd..f981ae4d 100644 --- a/api/resources/openapi.yml +++ b/api/resources/openapi.yml @@ -8,7 +8,8 @@ info: x-role: responsible developer description: 'Name Resolver (Name Lookup) service
This service takes lexical strings and attempts to map them to identifiers (CURIEs) from a vocabulary or ontology. An optional autocomplete mode (which assumes the query is incomplete) is available, - along with many other options. Given a preferred CURIE, the known synonyms of that CURIE can also be retrieved. + as is an exact mode (which requires the whole string to match a name or synonym), along with many other options. + Given a preferred CURIE, the known synonyms of that CURIE can also be retrieved. Multiple results may be returned representing possible conceptual matches, but all of the identifiers have been correctly normalized using the Node Normalization service.You can read more diff --git a/api/server.py b/api/server.py index df8c30c6..9bf24c9b 100755 --- a/api/server.py +++ b/api/server.py @@ -3,6 +3,8 @@ Queries are mostly sent to the underlying the NameRes Solr instance. """ +import asyncio +import html import json import logging import warnings @@ -12,7 +14,7 @@ from enum import Enum from typing import Dict, List, Union, Annotated, Optional -from fastapi import Body, FastAPI, Query +from fastapi import Body, FastAPI, HTTPException, Query from fastapi.responses import RedirectResponse import httpx from pydantic import BaseModel, Field @@ -26,6 +28,30 @@ # backups we used to ship called it name_lookup_shard1_replica_n1 instead (see status()). SOLR_CORE = os.getenv("SOLR_CORE", "name_lookup") +# The maximum number of Solr queries a single /bulk-lookup request may have in flight at once. +# bulk_lookup() runs its per-string lookups concurrently, and `strings` is unbounded, so without +# this a large bulk request would open one socket per string -- enough to exhaust this process's +# file descriptors and to stampede Solr. Raising this trades Solr load for bulk-lookup latency. +# +# Note that this bounds a *single* request, not the process: the Solr queries in flight across the +# whole service is this multiplied by the number of concurrent /bulk-lookup requests being served. +# 100 is set deliberately high on the assumption that Solr can take the strain, and should be +# revisited once we know the real request rate -- see TranslatorSRI/babel-validation#107. +# +# Clamped to at least 1: a value of 0 would produce a semaphore nobody can acquire, wedging every +# /bulk-lookup request forever with no error and no log line, which is a miserable thing to debug +# for what is usually a typo in a deployment's environment. +SOLR_MAX_CONCURRENT_LOOKUPS = max(1, int(os.getenv("SOLR_MAX_CONCURRENT_LOOKUPS", "100"))) + +# How long to wait for Solr before giving up on a single query, in seconds. +# +# This matters more than it used to. When bulk_lookup() ran its lookups sequentially, a stalled +# Solr connection held up one query; now that they run concurrently, one stalled connection can +# pin an otherwise-complete bulk request indefinitely while holding a semaphore slot. Set it to +# 0 to restore the previous behaviour of waiting forever. +SOLR_TIMEOUT_SECONDS = float(os.getenv("SOLR_TIMEOUT_SECONDS", "60")) +SOLR_TIMEOUT = SOLR_TIMEOUT_SECONDS if SOLR_TIMEOUT_SECONDS > 0 else None + app = FastAPI(**get_app_info()) logger = logging.getLogger(__name__) logging.basicConfig(level=os.getenv("LOGLEVEL", logging.INFO)) @@ -61,7 +87,7 @@ async def status_get() -> Dict: async def status() -> Dict: """ Return a dictionary containing status and count information for the underlying Solr instance. """ query_url = f"http://{SOLR_HOST}:{SOLR_PORT}/solr/admin/cores" - async with httpx.AsyncClient(timeout=None) as client: + async with httpx.AsyncClient(timeout=SOLR_TIMEOUT) as client: response = await client.get(query_url, params={ 'action': 'STATUS' }) @@ -241,7 +267,7 @@ async def name_lookup(curies) -> Dict[str, Dict]: "query": curie_filter, "limit": 1000000, } - async with httpx.AsyncClient(timeout=None) as client: + async with httpx.AsyncClient(timeout=SOLR_TIMEOUT) as client: response = await client.post(query, json=params) response.raise_for_status() response_json = response.json() @@ -257,6 +283,13 @@ async def name_lookup(curies) -> Dict[str, Dict]: return output +class ExactMatchMode(str, Enum): + """Controls exact-match behaviour in lookup queries.""" + label = "label" # match against preferred_name_exactish only + synonyms = "synonyms" # match against names_exactish only + any = "any" # match against either + + class LookupResult(BaseModel): curie:str label: str @@ -326,12 +359,18 @@ async def lookup_curies_get( )] = None, debug: Annotated[Union[DebugOptions, None], Query( description="Provide debugging information on the Solr query as described in Solr's debug parameters." - )] = 'none' + )] = 'none', + exact: Annotated[Optional[ExactMatchMode], Query( + description="Exact-match mode: 'label' matches the preferred name only, " + "'synonyms' matches any synonym, 'any' matches either. In every mode the " + "entire string must match, case-insensitively. " + "Omit for the default tokenized search." + )] = None, ) -> List[LookupResult]: """ Returns cliques with a name or synonym that contains a specified string. """ - return await lookup(string, autocomplete, highlighting, offset, limit, biolink_type, only_prefixes, exclude_prefixes, only_taxa, debug) + return await lookup(string, autocomplete, highlighting, offset, limit, biolink_type, only_prefixes, exclude_prefixes, only_taxa, debug, exact) @app.post("/lookup", @@ -390,12 +429,18 @@ async def lookup_curies_post( )] = None, debug: Annotated[Union[DebugOptions, None], Query( description="Provide debugging information on the Solr query as per Solr's debug parameter." - )] = 'none' + )] = 'none', + exact: Annotated[Optional[ExactMatchMode], Query( + description="Exact-match mode: 'label' matches the preferred name only, " + "'synonyms' matches any synonym, 'any' matches either. In every mode the " + "entire string must match, case-insensitively. " + "Omit for the default tokenized search." + )] = None, ) -> List[LookupResult]: """ Returns cliques with a name or synonym that contains a specified string. """ - return await lookup(string, autocomplete, highlighting, offset, limit, biolink_type, only_prefixes, exclude_prefixes, only_taxa, debug) + return await lookup(string, autocomplete, highlighting, offset, limit, biolink_type, only_prefixes, exclude_prefixes, only_taxa, debug, exact) async def lookup(string: str, @@ -408,6 +453,7 @@ async def lookup(string: str, exclude_prefixes: str = "", only_taxa: str = "", debug: DebugOptions = 'none', + exact: Optional[ExactMatchMode] = None, ) -> List[LookupResult]: """ Returns cliques with a name or synonym that contains a specified string. @@ -422,6 +468,17 @@ async def lookup(string: str, time_start = time.time_ns() + # autocomplete asks us to treat the last word as a prefix; exact asks us to match the whole + # string and nothing else. There is no sensible reading of the two together, so rather than + # silently ignoring one of them, say so. + if exact and autocomplete: + raise HTTPException( + status_code=400, + detail="autocomplete=true cannot be combined with exact matching: autocomplete treats " + "the final word as an incomplete prefix, while exact requires the entire string " + "to match. Please use one or the other.", + ) + # First, we strip and lowercase the query since all our indexes are case-insensitive. string_lc = string.strip().lower() @@ -432,7 +489,15 @@ async def lookup(string: str, # But the only issue we've actually run into so far has been the Windows smart # quote (https://github.com/NCATSTranslator/NameResolution/issues/176), so for now # let's detect and replace just those characters. - string_lc = re.sub(r"[“”]", '"', re.sub(r"[‘’]", "'", string_lc)) + # + # Deliberately not done in exact mode. The *_exactish fields are a KeywordTokenizer and a + # LowerCaseFilter, with no punctuation folding of their own, so the indexed value keeps + # whichever quote characters Babel emitted. Rewriting the query's quotes would therefore make + # exact mode search for a string the caller did not type, and would put any label containing a + # typographic quote permanently out of reach. The default search is unaffected either way, + # because StandardTokenizer discards the punctuation at index and query time alike. + if not exact: + string_lc = re.sub(r"[“”]", '"', re.sub(r"[‘’]", "'", string_lc)) # Do we have a search string at all? if string_lc == "": @@ -500,7 +565,9 @@ async def lookup(string: str, # Turn on highlighting if requested. inner_params = {} - if highlighting: + # In exact mode there is no scored query for Solr to highlight against (see below), so the + # highlighting is synthesized from the returned documents instead of asked for here. + if highlighting and not exact: inner_params.update({ # Highlighting "hl": "true", @@ -518,37 +585,72 @@ async def lookup(string: str, # Rather than returning the explain as a string, return it as structured JSON. inner_params['debug.explain.structured'] = 'true' - params = { - "query": { - "edismax": { - "query": query, - # qf = query fields, i.e. how should we boost these fields if they contain the same fields as the input. - # https://solr.apache.org/guide/solr/latest/query-guide/dismax-query-parser.html#qf-query-fields-parameter - "qf": "preferred_name_exactish^250 names_exactish^100 preferred_name^25 names^10", - # pf = phrase fields, i.e. how should we boost these fields if they contain the entire search phrase. - # https://solr.apache.org/guide/solr/latest/query-guide/dismax-query-parser.html#pf-phrase-fields-parameter - "pf": "preferred_name_exactish^300 names_exactish^200 preferred_name^30 names^20", - # Boosts - "bq": [], - "boost": [ - # The boost is multiplied with score -- calculating the log() reduces how quickly this increases - # the score for increasing clique identifier counts. - "log(sum(clique_identifier_count, 1))" - ], + if exact: + # Exact mode: bypass eDisMax entirely and match with a filter query against the *_exactish + # fields. Unlike the default query below, which matches the string's tokens in any order, + # this requires the whole string to match (case-insensitively -- see the exactish fieldType + # in the schema). A filter query is the right shape for that: there is nothing to score. + string_lc_escaped = string_lc.replace('\\', '\\\\').replace('"', '\\"') + if exact == ExactMatchMode.label: + exact_clause = f'preferred_name_exactish:"{string_lc_escaped}"' + elif exact == ExactMatchMode.synonyms: + exact_clause = f'names_exactish:"{string_lc_escaped}"' + else: # ExactMatchMode.any + exact_clause = ( + f'(preferred_name_exactish:"{string_lc_escaped}" OR names_exactish:"{string_lc_escaped}")' + ) + + # Marked uncached deliberately. Solr's filterCache is an entry count, not a size in bytes + # (solrconfig.xml sets 512 entries), and the entries it holds are shared, reusable filters + # like types: and taxa: that nearly every search benefits from. This clause is the opposite + # of that: one distinct entry per distinct search string. The workload exact mode exists to + # serve -- an NER pipeline resolving large numbers of *different* strings -- would therefore + # evict the whole cache on every request and slow down the ordinary search path as + # collateral damage, in exchange for a hit rate near zero on its own entries. Repeated + # identical exact lookups are still served from the queryResultCache, which is bounded by + # RAM rather than by entry count and caches the whole (query, filters, sort) result. + filters.append(f'{{!cache=false}}{exact_clause}') + params = { + "query": "*:*", + "filter": filters, + "sort": "clique_identifier_count DESC, curie_suffix ASC", + "limit": limit, + "offset": offset, + "fields": "*, score", + "params": inner_params, + } + else: + params = { + "query": { + "edismax": { + "query": query, + # qf = query fields, i.e. how should we boost these fields if they contain the same fields as the input. + # https://solr.apache.org/guide/solr/latest/query-guide/dismax-query-parser.html#qf-query-fields-parameter + "qf": "preferred_name_exactish^250 names_exactish^100 preferred_name^25 names^10", + # pf = phrase fields, i.e. how should we boost these fields if they contain the entire search phrase. + # https://solr.apache.org/guide/solr/latest/query-guide/dismax-query-parser.html#pf-phrase-fields-parameter + "pf": "preferred_name_exactish^300 names_exactish^200 preferred_name^30 names^20", + # Boosts + "bq": [], + "boost": [ + # The boost is multiplied with score -- calculating the log() reduces how quickly this increases + # the score for increasing clique identifier counts. + "log(sum(clique_identifier_count, 1))" + ], + }, }, - }, - "sort": "score DESC, clique_identifier_count DESC, curie_suffix ASC", - "limit": limit, - "offset": offset, - "filter": filters, - "fields": "*, score", - "params": inner_params, - } + "sort": "score DESC, clique_identifier_count DESC, curie_suffix ASC", + "limit": limit, + "offset": offset, + "filter": filters, + "fields": "*, score", + "params": inner_params, + } logger.debug(f"Query: {json.dumps(params, indent=2)}") time_solr_start = time.time_ns() query_url = f"http://{SOLR_HOST}:{SOLR_PORT}/solr/{SOLR_CORE}/select" - async with httpx.AsyncClient(timeout=None) as client: + async with httpx.AsyncClient(timeout=SOLR_TIMEOUT) as client: response = await client.post(query_url, json=params) if response.status_code >= 300: logger.error("Solr REST error: %s", response.text) @@ -571,7 +673,29 @@ async def lookup(string: str, preferred_matches = [] synonym_matches = [] - if doc['id'] in highlighting_response: + if exact and highlighting: + # Solr did not highlight anything for us: exact mode matches with a filter query + # against fields that are not stored, so there is no scored query and nothing for the + # highlighter to mark up. Synthesize the same shape from the documents instead, so that + # a caller reading `highlighting` does not have to care which mode produced it. + # + # Every match in exact mode is a whole-value match, so the "highlighted" form of a + # matching name is simply the entire name wrapped in the same tags Solr would have + # used. Escape it first, since we ask Solr for hl.encoder=html in the default path. + def mark(value: str) -> str: + return f"{html.escape(value)}" + + if exact in {ExactMatchMode.label, ExactMatchMode.any}: + preferred_name = doc.get("preferred_name", "") + if preferred_name.lower() == string_lc: + preferred_matches.append(mark(preferred_name)) + + if exact in {ExactMatchMode.synonyms, ExactMatchMode.any}: + synonym_matches.extend( + mark(name) for name in doc.get("names", []) if name.lower() == string_lc + ) + + elif doc['id'] in highlighting_response: matches = highlighting_response[doc['id']] # We order exactish matches before token matches. @@ -619,7 +743,7 @@ async def lookup(string: str, time_end = time.time_ns() logger.info(f"Lookup query to Solr for {json.dumps(string)} " + - f"(autocomplete={autocomplete}, highlighting={highlighting}, offset={offset}, limit={limit}, biolink_types={biolink_types}, only_prefixes={only_prefixes}, exclude_prefixes={exclude_prefixes}, only_taxa={only_taxa}): " + f"(autocomplete={autocomplete}, highlighting={highlighting}, offset={offset}, limit={limit}, biolink_types={biolink_types}, only_prefixes={only_prefixes}, exclude_prefixes={exclude_prefixes}, only_taxa={only_taxa}, exact={exact}): " f"took {(time_end - time_start)/1_000_000:.2f}ms (with {(time_solr_end - time_solr_start)/1_000_000:.2f}ms waiting for Solr)" ) @@ -686,6 +810,13 @@ class NameResQuery(BaseModel): 'none', description="Provide debugging information on the Solr query as per Solr's debug parameter." ) + exact: Optional[ExactMatchMode] = Field( + None, + description="Exact-match mode: 'label' matches the preferred name only, " + "'synonyms' matches any synonym, 'any' matches either. In every mode the " + "entire string must match, case-insensitively. " + "Omit (or null) for the default tokenized search.", + ) @app.post("/bulk-lookup", @@ -698,19 +829,31 @@ class NameResQuery(BaseModel): ) async def bulk_lookup(query: NameResQuery) -> Dict[str, List[LookupResult]]: time_start = time.time_ns() - result = {} - for string in query.strings: - result[string] = await lookup( - string, - query.autocomplete, - query.highlighting, - query.offset, - query.limit, - query.biolink_types, - query.only_prefixes, - query.exclude_prefixes, - query.only_taxa, - query.debug) + + # Bounded so that a single large request can't open a socket per string; see + # SOLR_MAX_CONCURRENT_LOOKUPS. + semaphore = asyncio.Semaphore(SOLR_MAX_CONCURRENT_LOOKUPS) + + async def do_lookup(string: str): + async with semaphore: + results = await lookup( + string, + query.autocomplete, + query.highlighting, + query.offset, + query.limit, + query.biolink_types, + query.only_prefixes, + query.exclude_prefixes, + query.only_taxa, + query.debug, + query.exact, + ) + return string, results + + pairs = await asyncio.gather(*[do_lookup(s) for s in query.strings]) + result = dict(pairs) + time_end = time.time_ns() logger.info(f"Bulk lookup query for {len(query.strings)} strings ({query}): took {(time_end - time_start)/1_000_000:.2f}ms") return result diff --git a/documentation/API.md b/documentation/API.md index 5311a2d5..9dea8be7 100644 --- a/documentation/API.md +++ b/documentation/API.md @@ -108,7 +108,9 @@ We are currently working on supporting ## Search endpoints -The search endpoints allow you to search for concepts by a fragment of a name or synonym. These endpoints use Solr's extended Dismax query parser to search for matches across preferred names and synonyms, with support for highlighting, filtering, pagination, and debugging. +The search endpoints allow you to search for concepts by a fragment of a name or synonym. By default these endpoints use Solr's extended Dismax query parser to search for matches across preferred names and synonyms, with support for highlighting, filtering, pagination, and debugging. Setting the `exact` parameter switches them to whole-string exact matching instead (see [Exact matching](#exact-matching)). + +The default search is *tokenized*, not *fuzzy*: your search string is broken into tokens, and a concept matches if its name or synonyms contain those tokens, in any order and not necessarily adjacent. Word order and adjacency are rewarded with a higher score rather than required, which is why `disease Parkinson` still finds `Parkinson disease`. What the default search does **not** do is tolerate misspellings: there is no edit-distance matching, so `parkinsen` finds nothing. (Solr's fuzzy `~` operator is escaped out of the query string along with the other special characters, so it cannot be requested either.) The one exception is `autocomplete=true`, which treats the *final* token as a prefix so that a half-typed word still matches. ### `/lookup` @@ -128,6 +130,7 @@ Search for cliques by a fragment of a name or synonym. - `exclude_prefixes` (optional, string): Pipe-separated, case-sensitive list of CURIE prefixes to exclude (e.g., `UMLS|EFO`). Results with matching prefixes will be filtered out. - `only_taxa` (optional, string): Pipe-separated, case-sensitive list of taxa to filter to (e.g., `NCBITaxon:9606|NCBITaxon:10090|NCBITaxon:10116|NCBITaxon:7955`). Results without a specified taxon or with a matching taxon will be included. - `debug` (optional, string, one of: `none`, `query`, `timing`, `results`, `all`, default: `none`): Return debugging information from the underlying Solr query. See [Solr debug documentation](https://solr.apache.org/guide/solr/latest/query-guide/common-query-parameters.html#debug-parameter) for details. +- `exact` (optional, string, one of: `label`, `synonyms`, `any`): Require the whole search string to match, instead of matching its tokens individually. See [Exact matching](#exact-matching) below. Omit for the default tokenized search. **Returns:** A list of `LookupResult` objects, each containing: - `curie`: The CURIE of the concept. @@ -183,7 +186,8 @@ Search for cliques for multiple strings in a single request. "only_prefixes": "", "exclude_prefixes": "", "only_taxa": "", - "debug": "none" + "debug": "none", + "exact": null } ``` @@ -230,6 +234,38 @@ POST `/bulk-lookup` with body: **Notes:** - This endpoint is useful for batch processing multiple queries at once, which can be more efficient than making multiple `/lookup` requests. - All results for a given string share the same filter and search parameters. +- The individual lookups behind a single request are sent to Solr concurrently, up to a bounded number at a time (`SOLR_MAX_CONCURRENT_LOOKUPS`, default 100). Results are still returned keyed by the input string, so the response does not depend on the order in which they complete. Note that the bound applies per request, so the total number of queries this service has in flight against Solr is that limit multiplied by the number of bulk lookups being served at once. + +### Exact matching + +Both `/lookup` and `/bulk-lookup` accept an `exact` parameter, which replaces the default tokenized eDisMax search with whole-string matching against the `preferred_name_exactish` and `names_exactish` fields. These fields are indexed with a keyword tokenizer and a lowercase filter, so the *entire* string must match, but matching is case-insensitive. A search for `parkinson` will not match the label `parkinsonian disorder`, whereas `Parkinsonian Disorder` will. + +| `exact` | Matches against | +| --- | --- | +| `label` | The preferred name only (`preferred_name_exactish`). | +| `synonyms` | The synonyms only (`names_exactish`). | +| `any` | Either the preferred name or a synonym. | +| *(omitted)* | Nothing changes: the usual tokenized eDisMax search is used. | + +Note that a concept's preferred name is not necessarily one of its synonyms, so `label` and `synonyms` can genuinely disagree. For example, HP:0001300 has the preferred name `parkinsonian disorder` and the single synonym `Parkinsonian disease`; searching for `parkinsonian disorder` with `exact=label` finds it, but with `exact=synonyms` it does not. + +Exact matching is implemented as a Solr filter query, which is cheaper than the equivalent tokenized search because there is nothing to score: no eDisMax parsing, no phrase or field boosts, just a term lookup against a field that holds each name as a single token. It is intended for callers such as named entity recognition pipelines that need to resolve large numbers of exact strings. + +That filter is deliberately marked uncached (`{!cache=false}`). Solr's filterCache is bounded by entry count rather than memory, and it holds the shared, reusable filters — `types:`, `taxa:`, `curie:` — that nearly every search benefits from. An exact-match clause is one distinct entry per distinct search string, so caching it would let a single bulk NER request evict the entire cache and slow down the ordinary search path, in exchange for a hit rate near zero on its own entries. Repeated *identical* exact lookups are still served from the queryResultCache, which caches the whole result and is bounded by RAM. + +Because there is no relevance score to rank by in exact mode, results are sorted by `clique_identifier_count` (descending) and then CURIE suffix (ascending), rather than by score. The `score` field is still present in each result, but it carries no ranking information. + +#### Exact matching and the other parameters + +- **`autocomplete` cannot be used with `exact`.** The two contradict each other — autocomplete treats the final word as an incomplete prefix, while exact requires the whole string to match — so the combination is rejected with a `400` rather than one of them being silently ignored. +- **Typographic quotes are not rewritten in exact mode.** The default search folds `‘ ’ “ ”` to their ASCII equivalents, since input is sometimes mangled by Windows and the tokenizer discards the punctuation regardless. Exact mode deliberately does not: the `*_exactish` fields keep whatever characters Babel emitted, so rewriting the query would search for a string you did not type and would put any label containing a typographic quote out of reach. Search for the string exactly as it appears in the data. +- **`highlighting` works, and always marks up the whole value.** Every match in exact mode is a whole-value match, so a highlighted result is the entire matching name wrapped in ``…`` — `parkinsonian disorder` matched with `exact=label` returns `{"labels": ["parkinsonian disorder"], "synonyms": []}`. The name is returned in its own capitalisation, not the query's, so a case-insensitive match is still visible as one. + +**Example:** + +``` +GET /lookup?string=parkinsonian%20disorder&exact=label +``` ## Lookup endpoints diff --git a/documentation/Deployment.md b/documentation/Deployment.md index 45424e30..7734e778 100644 --- a/documentation/Deployment.md +++ b/documentation/Deployment.md @@ -105,6 +105,8 @@ NameRes can be configured by setting environmental variables: * `SOLR_HOST` and `SOLR_PORT`: Hostname and port for the Solr database containing NameRes information. * `SOLR_CORE`: The Solr core to query (defaults to `name_lookup`). +* `SOLR_MAX_CONCURRENT_LOOKUPS`: The greatest number of Solr queries a single `/bulk-lookup` request may have in flight at once (defaults to `100`, and is clamped to at least `1`). The bound is per request, so the queries this service has in flight against Solr is this multiplied by the number of bulk lookups being served at once. +* `SOLR_TIMEOUT_SECONDS`: How long to wait for any one Solr query before giving up (defaults to `60`). Set it to `0` to wait indefinitely. * `SERVER_NAME`: The name of this server (defaults to `infores:sri-name-resolver`) * `SERVER_ROOT`: The server root (defaults to `/`) * `MATURITY_VALUE`: How mature is this NameRes (defaults to `maturity`, e.g. `development`) diff --git a/tests/test_exact_mode.py b/tests/test_exact_mode.py new file mode 100644 index 00000000..a6a9e7db --- /dev/null +++ b/tests/test_exact_mode.py @@ -0,0 +1,223 @@ +# This file tests the exact-match mode (the `exact` parameter) on the lookup and bulk_lookup +# endpoints. Tests for the default, tokenized search live in test_service.py. +import json +import logging + +from api.server import app +from fastapi.testclient import TestClient + +# Turn on debugging for tests. +logging.basicConfig(level=logging.DEBUG) + +# HP:0001300 has preferred_name="parkinsonian disorder" and names=["Parkinsonian disease"]. +# The preferred_name is NOT in names, making it a good test case for label vs. synonyms exact mode. + +def test_exact_label_match(): + client = TestClient(app) + # "parkinsonian disorder" is the preferred label for HP:0001300 — label mode should find it. + response = client.post("/lookup", params={'string': 'parkinsonian disorder', 'exact': 'label', 'limit': 100}) + results = response.json() + curies = [r['curie'] for r in results] + assert 'HP:0001300' in curies + + # "Parkinsonian disease" is only a synonym (names entry), not the preferred label — label mode should NOT find it. + response = client.post("/lookup", params={'string': 'Parkinsonian disease', 'exact': 'label', 'limit': 100}) + results = response.json() + curies = [r['curie'] for r in results] + assert 'HP:0001300' not in curies + +def test_exact_synonyms_match(): + client = TestClient(app) + # "Parkinsonian disease" is a synonym (names entry) for HP:0001300 — synonyms mode should find it. + response = client.post("/lookup", params={'string': 'Parkinsonian disease', 'exact': 'synonyms', 'limit': 100}) + results = response.json() + curies = [r['curie'] for r in results] + assert 'HP:0001300' in curies + + # "parkinsonian disorder" is the preferred_name but NOT in names for HP:0001300 — synonyms mode should NOT find it. + response = client.post("/lookup", params={'string': 'parkinsonian disorder', 'exact': 'synonyms', 'limit': 100}) + results = response.json() + curies = [r['curie'] for r in results] + assert 'HP:0001300' not in curies + +def test_exact_any_match(): + client = TestClient(app) + # exact=any should match on either the preferred label or a synonym. + response = client.post("/lookup", params={'string': 'parkinsonian disorder', 'exact': 'any', 'limit': 100}) + curies = [r['curie'] for r in response.json()] + assert 'HP:0001300' in curies + + response = client.post("/lookup", params={'string': 'Parkinsonian disease', 'exact': 'any', 'limit': 100}) + curies = [r['curie'] for r in response.json()] + assert 'HP:0001300' in curies + +def test_exact_no_partial_match(): + client = TestClient(app) + # "parkinson" is only a substring of known terms — exact mode must return no match for HP:0001300. + response = client.post("/lookup", params={'string': 'parkinson', 'exact': 'any', 'limit': 100}) + curies = [r['curie'] for r in response.json()] + assert 'HP:0001300' not in curies + +def test_exact_bulk_lookup(): + client = TestClient(app) + params = { + 'strings': ['parkinsonian disorder', 'Parkinsonian disease', 'no match term xyz'], + 'exact': 'any', + 'limit': 10, + } + response = client.post("/bulk-lookup", json=params) + results = response.json() + + assert set(results.keys()) == {'parkinsonian disorder', 'Parkinsonian disease', 'no match term xyz'} + assert 'HP:0001300' in [r['curie'] for r in results['parkinsonian disorder']] + assert 'HP:0001300' in [r['curie'] for r in results['Parkinsonian disease']] + assert results['no match term xyz'] == [] + +def test_exact_rejects_autocomplete(): + client = TestClient(app) + # autocomplete treats the final word as a prefix and exact requires the whole string to match, + # so the combination has no sensible meaning and must be refused rather than silently resolved. + response = client.get("/lookup", params={ + 'string': 'parkinsonian disorder', + 'exact': 'label', + 'autocomplete': 'true', + }) + assert response.status_code == 400 + assert 'autocomplete' in response.json()['detail'] + + # The same request without autocomplete is fine. + response = client.get("/lookup", params={'string': 'parkinsonian disorder', 'exact': 'label'}) + assert response.status_code == 200 + + # ...and so is autocomplete without exact. + response = client.get("/lookup", params={'string': 'parkinsonian dis', 'autocomplete': 'true'}) + assert response.status_code == 200 + +def test_exact_highlighting_returns_the_whole_matched_value(): + client = TestClient(app) + + # HP:0001300 is preferred_name="parkinsonian disorder", names=["Parkinsonian disease"]. + response = client.get("/lookup", params={ + 'string': 'parkinsonian disorder', + 'exact': 'label', + 'highlighting': 'true', + 'limit': 100, + }) + result = next(r for r in response.json() if r['curie'] == 'HP:0001300') + # The whole value matched, so the whole value comes back marked up. + assert result['highlighting']['labels'] == ['parkinsonian disorder'] + assert result['highlighting']['synonyms'] == [] + + # Matching on a synonym highlights the synonym, in its original case rather than the query's. + response = client.get("/lookup", params={ + 'string': 'parkinsonian disease', + 'exact': 'synonyms', + 'highlighting': 'true', + 'limit': 100, + }) + result = next(r for r in response.json() if r['curie'] == 'HP:0001300') + assert result['highlighting']['labels'] == [] + assert result['highlighting']['synonyms'] == ['Parkinsonian disease'] + + # Without highlighting the field stays empty, exactly as in the default search. + response = client.get("/lookup", params={ + 'string': 'parkinsonian disorder', + 'exact': 'label', + 'limit': 100, + }) + result = next(r for r in response.json() if r['curie'] == 'HP:0001300') + assert result['highlighting'] == {} + +def test_exact_does_not_rewrite_smart_quotes(): + client = TestClient(app) + + # The default search folds typographic quotes to ASCII, because the input may have been + # mangled by Windows (issue #176) and StandardTokenizer discards the punctuation anyway. + response = client.get("/lookup", params={'string': '‘parkinson’', 'limit': 100}) + assert response.status_code == 200 + + # Exact mode must not do that folding: the *_exactish fields keep whatever characters Babel + # emitted, so rewriting the query would search for a string the caller never typed. MONDO:0005180 + # carries the ASCII "Parkinson's disease", which the ASCII query finds... + response = client.get("/lookup", params={ + 'string': "Parkinson's disease", 'exact': 'synonyms', 'limit': 100, + }) + assert 'MONDO:0005180' in [r['curie'] for r in response.json()] + + # ...and the typographic-apostrophe query does not, since that is a different string. + response = client.get("/lookup", params={ + 'string': 'Parkinson’s disease', 'exact': 'synonyms', 'limit': 100, + }) + assert 'MONDO:0005180' not in [r['curie'] for r in response.json()] + + +def test_exact_filter_query_is_not_cached(): + """ + The exactish clause must stay marked uncached. + + It is one distinct filterCache entry per distinct search string, and the filterCache is bounded + by entry count (512), so caching it would evict the shared types:/taxa:/curie: filters that the + ordinary search path relies on -- for a hit rate near zero on exactly the workload exact mode + exists to serve. See the comment in lookup(). + """ + client = TestClient(app) + response = client.get("/lookup", params={ + 'string': 'parkinsonian disorder', + 'exact': 'label', + 'debug': 'query', + 'limit': 100, + }) + assert response.status_code == 200 + results = response.json() + assert results, "Expected at least one result to carry the debug information." + + # Dump the whole debug structure rather than reaching for a particular Solr key, since the + # question is only whether the filter went to Solr marked uncached. + debug_info = json.dumps(results[0]['debug']) + assert 'preferred_name_exactish' in debug_info, \ + f"Expected the exactish filter query in the debug output, got: {debug_info}" + assert 'cache=false' in debug_info, \ + f"Expected the exactish filter query to be marked uncached, got: {debug_info}" + + +def test_exact_combines_with_the_other_filters(): + """ + The exactish clause is appended to the same filter list as biolink_type, only_taxa and the + prefix filters, so they have to keep working together -- filtering an exact lookup to a type is + exactly what an NER pipeline resolving into a known category would do. + """ + client = TestClient(app) + + base = {'string': 'parkinsonian disorder', 'exact': 'label', 'limit': 100} + assert 'HP:0001300' in [r['curie'] for r in client.get("/lookup", params=base).json()] + + # HP:0001300 is typed Disease -- not PhenotypicFeature, despite the HP prefix -- so filtering + # to that type keeps the result... + response = client.get("/lookup", params={**base, 'biolink_type': 'biolink:Disease'}) + assert 'HP:0001300' in [r['curie'] for r in response.json()] + + # ...and filtering to an unrelated type removes it without disturbing the exact match itself. + response = client.get("/lookup", params={**base, 'biolink_type': 'biolink:Gene'}) + assert 'HP:0001300' not in [r['curie'] for r in response.json()] + + # The prefix filters apply the same way. + response = client.get("/lookup", params={**base, 'only_prefixes': 'MONDO'}) + assert 'HP:0001300' not in [r['curie'] for r in response.json()] + + response = client.get("/lookup", params={**base, 'exclude_prefixes': 'HP'}) + assert 'HP:0001300' not in [r['curie'] for r in response.json()] + + +def test_exact_rejects_autocomplete_in_bulk_lookup(): + """ + The 400 is raised inside lookup(), which bulk_lookup() now runs under asyncio.gather(), so + check the exception still surfaces as a 400 rather than being swallowed into a 500. + """ + client = TestClient(app) + response = client.post("/bulk-lookup", json={ + 'strings': ['parkinsonian disorder', 'Parkinson disease'], + 'exact': 'any', + 'autocomplete': True, + }) + assert response.status_code == 400 + assert 'autocomplete' in response.json()['detail'] diff --git a/tests/test_service.py b/tests/test_service.py index 2fa9a242..7c67e44b 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -1,5 +1,6 @@ import logging +import api.server from api.server import app from fastapi.testclient import TestClient @@ -14,7 +15,6 @@ def test_simple_check(): #There are more than 10, but it should cut off at 10 if we don't give it a max? assert len(syns) == 10 - def test_empty(): """ Checks that calling NameRes without an input string return an empty list. """ client = TestClient(app) @@ -22,7 +22,6 @@ def test_empty(): syns = response.json() assert len(syns) == 0 - def test_limit(): client = TestClient(app) params = {'string': 'alzheimer', 'limit': 1} @@ -34,7 +33,6 @@ def test_limit(): syns = response.json() assert len(syns) == 30 - def test_type_subsetting(): client = TestClient(app) #Get everything with Parkinson (57) @@ -91,7 +89,6 @@ def test_structure(): assert syns[0]["label"] == 'BACE1 inhibitor' assert syns[0]["types"] == ["biolink:NamedThing"] - def test_autocomplete(): client = TestClient(app) params = {'string': 'beta-secretase', 'autocomplete': 'true'} @@ -158,7 +155,6 @@ def test_windows_smartquotes(): assert syns[0]['label'] == 'Alzheimer disease' assert syns[0]['types'][0] == 'biolink:Disease' - def test_bulk_lookup(): client = TestClient(app) params = { @@ -190,7 +186,6 @@ def test_bulk_lookup(): assert results['Parkinson'][0]['curie'] == 'MONDO:0005180' assert results['Parkinson'][0]['label'] == "Parkinson disease" - def test_synonyms(): """ Test the /synonyms endpoints -- these are used to look up all the information we know about a preferred CURIE. @@ -259,4 +254,100 @@ def test_only_taxa_queries(): }) results_ftd_disease_with_only_taxon = response.json() assert len(results_ftd_disease_with_only_taxon) == 1 - assert results_ftd_disease_with_only_taxon[0]['curie'] == 'MONDO:0010857' \ No newline at end of file + assert results_ftd_disease_with_only_taxon[0]['curie'] == 'MONDO:0010857' + +def test_bulk_lookup_beyond_concurrency_limit(monkeypatch): + """ + bulk_lookup() runs its per-string lookups concurrently behind a semaphore, so exercise it with + more strings than the semaphore allows in flight at once: every string must still come back + keyed to its own results, however the lookups interleave. This is a property of bulk lookup + rather than of exact matching; exact=label is used only to make each string's result definite. + + The limit is patched down rather than sending SOLR_MAX_CONCURRENT_LOOKUPS-worth of real strings, + because the default (100) is larger than the number of distinct labels in the test data. Patching + the module attribute works because bulk_lookup() builds its semaphore per request, at call time. + """ + client = TestClient(app) + monkeypatch.setattr(api.server, "SOLR_MAX_CONCURRENT_LOOKUPS", 3) + + expected = { + 'parkinsonian disorder': 'HP:0001300', + 'Resting tremor': 'HP:0002322', + 'juvenile-onset Parkinson disease': 'MONDO:0000828', + 'postencephalitic Parkinson disease': 'MONDO:0001945', + 'Parkinson disease': 'MONDO:0005180', + 'secondary Parkinson disease': 'MONDO:0006966', + 'Alzheimer disease type 1': 'MONDO:0007088', + 'Alzheimer disease 2': 'MONDO:0007089', + 'Lewy body dementia': 'MONDO:0007488', + 'dystonia 5': 'MONDO:0007495', + 'dystonia 12': 'MONDO:0007496', + 'antiparkinson agent': 'CHEBI:48407', + 'BACE1 inhibitor': 'CHEBI:74925', + } + assert len(expected) > api.server.SOLR_MAX_CONCURRENT_LOOKUPS, \ + "This test is only meaningful with more strings than can be looked up concurrently." + + response = client.post("/bulk-lookup", json={ + 'strings': list(expected.keys()), + 'exact': 'label', + 'limit': 100, + }) + results = response.json() + + assert set(results.keys()) == set(expected.keys()) + for string, curie in expected.items(): + assert curie in [r['curie'] for r in results[string]], \ + f"Expected {curie} in the results for {string!r}, got {results[string]}" + + +# The default (non-exact) search is tokenized, not fuzzy. These two tests pin down that distinction, +# since documentation/API.md makes a promise about it that is easy to break by changing the query or +# the schema's field types: word order is not required, but misspellings are not tolerated either. + +def test_default_search_ignores_word_order(): + client = TestClient(app) + # MONDO:0005180 is "Parkinson disease". The tokens may arrive in any order. + response = client.get("/lookup", params={'string': 'disease Parkinson', 'limit': 100}) + curies = [r['curie'] for r in response.json()] + assert 'MONDO:0005180' in curies + +def test_default_search_does_not_tolerate_misspellings(): + client = TestClient(app) + # No document contains the token "parkinsen", and there is no edit-distance matching to bridge + # the gap to "parkinson", so this must find nothing at all. + response = client.get("/lookup", params={'string': 'parkinsen', 'limit': 100}) + assert response.json() == [] + + # The same string with the typo corrected does match, so the miss above is the spelling and not + # some other property of the query. + response = client.get("/lookup", params={'string': 'parkinson', 'limit': 100}) + assert len(response.json()) > 0 + +def test_solr_settings_are_sane(): + # A concurrency limit of 0 would make every bulk lookup wait on a semaphore nobody can acquire. + assert api.server.SOLR_MAX_CONCURRENT_LOOKUPS >= 1 + # A stalled Solr connection must not be able to pin a bulk request forever by default. + assert api.server.SOLR_TIMEOUT is None or api.server.SOLR_TIMEOUT > 0 + + +def test_concurrency_limit_is_clamped_to_at_least_one(monkeypatch): + """ + SOLR_MAX_CONCURRENT_LOOKUPS=0 would build a semaphore nobody can ever acquire, hanging every + bulk lookup with no error and no log line. The clamp runs at import, so this has to reload the + module to exercise it. + """ + import importlib + + try: + monkeypatch.setenv("SOLR_MAX_CONCURRENT_LOOKUPS", "0") + importlib.reload(api.server) + assert api.server.SOLR_MAX_CONCURRENT_LOOKUPS == 1 + + monkeypatch.setenv("SOLR_MAX_CONCURRENT_LOOKUPS", "25") + importlib.reload(api.server) + assert api.server.SOLR_MAX_CONCURRENT_LOOKUPS == 25 + finally: + # Restore the module for whatever runs next, since reload mutates it in place. + monkeypatch.delenv("SOLR_MAX_CONCURRENT_LOOKUPS", raising=False) + importlib.reload(api.server)