Repository navigation
Update OpenAPI for SmartAPI registration and preserve multi-Babel behavior - #58
Merged
Merged
Conversation
The document was copied from NodeNorm Redis and never pruned, so it advertised TRAPI 1.5 (info.x-trapi), carried 145 schemas no path referenced, named RENCI's developer as the contact and sent readers to /get_curie_prefixes, which this service has never served. It also failed openapi-spec-validator: the /get_allowed_conflations response had no description, and FilterKgraphTopNParameters.max_edges declared maximum 0 (NCATSTranslator/NodeNormalization#409). Rewritten from scratch against build_handlers(): the six routes this service registers, nothing else. Responses that were `schema: {}` now have schemas (NormalizedNodes, SetIDResponse), the FastAPI 422 responses are replaced by the 400 the Tornado handlers raise, and the examples are taken from the live CI instance. The servers block lists the ci and test deployments at ITRB instead of RENCI. Contact is Will Liu and the team is DOGSURF (#35); infores and termsOfService are unchanged for now. info.version is bumped to 1.1.0 because /status reports it and the response shapes change in the next commit. handlers/curie_prefix.py was an unregistered copy of the semantic-types handler (its class is even named SemanticTypeHandler), so it goes too. Refs #35, #36 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GET /get_allowed_conflations finished a bare list, and Tornado's write()
refuses a top-level list ("Lists not accepted for security reasons"),
so it returned HTTP 500 on every deployment. It now returns
{"conflations": [...]}, which is also what NodeNorm Redis returns.
GET /get_semantic_types lower-cased every class ("biolink:namedthing");
NodeNorm Redis returns the Biolink class URI as is ("biolink:NamedThing").
/openapi.json now redirects to /webapp/openapi.json so the FastAPI
default address used by NodeNorm Redis, SmartAPI and Babel Validation
works here too; it is excluded from OpenTelemetry like the other static
paths.
The POST set-ID test posted a single object where the handler expects
a list of sets, and the normalized-nodes docstring showed a "curie" key
where the handler reads "curies"; both corrected. POST /get_setid still
answers with an object keyed "0", "1", ... (the same Tornado limitation);
returning a list as NodeNorm Redis does is a breaking change, so it is
left to its own pull request.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The document is hand-written and served as a static file, so these tests are what stops it drifting: the spec validates; its paths are exactly the routes build_handlers() registers; each path documents the GET/POST methods its handler defines; each GET documents the query parameters its handler reads (found by scanning the handler source for get_argument calls); and the SetIDResponse schema lists the dataclass's fields. Each was checked to fail when the spec is mutated. Also a regression test for /get_allowed_conflations, which returned HTTP 500 until the previous commit: it mounts just that handler and validates the body against the documented ConflationList schema. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
It ran only against the su10 Elasticsearch host before. This mounts the handler alone with a fake Elasticsearch client and validates the body against the documented schema. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The dot in each redirect pattern was unescaped, so /openapiXjson and /faviconXico redirected too. The OpenTelemetry exclusions already escape it. A test now checks the /openapi.json redirect and that a near-miss path is a 404. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ment
The query-parameter check scanned only get() for get_argument calls, so
a parameter read in a helper method would drift unnoticed; it now scans
the whole handler class. A new check compares the fields each post()
reads with .get("...") against the properties of the documented request
body (CurieList, and the SetIDQuery items of the /get_setid array).
Both were confirmed to fail when a field is removed from the document or
a parameter is renamed in a helper.
The checks still only see literal names; the README now says so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
generate_setid() returned the SetIDResponse dataclass itself on the invalid-conflation and no-normalized-CURIEs paths. Tornado can only write dicts, so GET /get_setid?conflation=Bogus answered 500 and the documented `error` field never reached anyone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The body has to be a JSON array of {curies, conflations} objects. A
single object (the shape the old document described) passed the length
check and then failed with an AttributeError, and invalid JSON failed in
json.loads; both were 500s. They are now 400s that say what shape the
body must have.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The index mapping (for the Babel version) was read before the try
block, so an unreachable cluster raised and /status answered 500
instead of the documented {"status": "error"}. The read now happens
inside the try; babel_version and babel_version_url are null when the
mapping couldn't be read, and the document says so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The patterns were written r"/get_setid?", r"/status?" and so on. In a Tornado route that ? makes the last character optional, so /get_seti and /statu reached the handlers though the OpenAPI document lists only the full paths. test_openapi.py hid this by stripping the ? before comparing routes with the document; it now compares them as written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The request-body check read only post()'s source, though the README says fields are found by scanning the handler, and the query-parameter check already scans the whole class. A body field read in a helper method was either missed or reported as undocumented. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The methods check compared only GET and POST, so the HEAD handler on /get_allowed_conflations, which the document didn't list, went unnoticed. It now compares every method Tornado routes except OPTIONS (answered by the base handler for CORS), and the document lists HEAD. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tests/test_openapi.py imports it directly; it was only installed because openapi-spec-validator happens to depend on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
parse_boolean() treated every string except "false" as true, so
conflate=0 or conflate=no turned conflation on. The flags now accept the
strings FastAPI/Pydantic accepted in NodeNorm Redis (true/false, 1/0,
yes/no, on/off, t/f, y/n, in any case) and answer 400 for anything else,
including an empty value. POST bodies go through the same check, so
{"conflate": "false"} is no longer truthy. The document lists the
accepted values.
This is a behavior change: a caller sending an unrecognized value now
gets a 400 where it used to get conflation silently switched on.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Covers the second path #47 describes: a set whose CURIEs normalize to nothing. get_normalized_nodes() is patched out, so it needs no host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
https://nodenorm-es.transltr.io/ is now live. It goes first, since Swagger UI and generated clients default to the first server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
json.loads() raises UnicodeDecodeError rather than JSONDecodeError for bytes that aren't valid UTF-8, so such a body escaped the check and became a 500, though the document promises a 400 for a malformed body. The decoding moves to a load_json_body() helper on the base handler so POST /get_normalized_nodes can use it too. Raised by Copilot on #42. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The handler called json.loads() with no error handling and then .get() on the result, so a body that wasn't JSON, wasn't UTF-8, or was an array instead of an object all became 500s. It now uses load_json_body() and checks for an object, and the document lists these under the 400 response. Same cause as the UTF-8 case Copilot raised for POST /get_setid on #42. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_document_is_valid_openapi() reads the file from the package, so a broken /webapp/ static route or /openapi.json redirect, or a wrong Content-Type, went unnoticed. The new test fetches /openapi.json through build_handlers(), follows the redirect, and validates the body. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
biolink/information-resource-registry#140 added infores:nodenorm for NodeNorm ES and renamed infores:sri-node-normalizer to NodeNorm Redis, so the two services no longer share an infores in SmartAPI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep multi-Babel selectors, selected-version headers, and the current status handler with HTTP 503 health failures. Update the OpenAPI sync checks for shared parameter references and per-method Tornado query readers, validate both release response shapes, and advance the API version to 1.2.0. Validation: 290 tests passed, with four Elasticsearch integration tests deselected; 12 subtests passed.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The equivalent-identifier schema rejects null types returned when individual_types=true and a clique’s source type is missing, leaving the response contract inconsistent with handler behavior.
1 open issue
What changed in this PR
Updates the Elasticsearch-backed NodeNorm API for SmartAPI registration while preserving multi-Babel release selection, response headers, and the existing status contract.
Changes:
- Replaces stale OpenAPI metadata and unused TRAPI schemas with endpoint contracts for API version 1.2.0.
- Corrects response serialization, semantic-type casing, input handling, and route matching; exposes
/openapi.json. - Adds offline schema validation and regression coverage, including multi-Babel responses.
| File | Description |
|---|---|
tests/test_set_identifiers.py |
Aligns POST requests and expectations with the batch contract. |
tests/test_openapi.py |
Adds document, handler-contract, and offline response validation. |
tests/test_babel_endpoints.py |
Updates semantic-type expectations across releases. |
tests/test_application.py |
Updates the exact status-route expectation. |
src/nodenorm/webapp/openapi.json |
Refreshes metadata, schemas, and endpoint contracts for version 1.2.0. |
src/nodenorm/handlers/set_identifiers.py |
Validates body structure and serializes early returns as dictionaries. |
src/nodenorm/handlers/semantic_types.py |
Preserves Biolink class casing. |
src/nodenorm/handlers/normalized_nodes.py |
Validates JSON body structure and parses boolean flags strictly. |
src/nodenorm/handlers/curie_prefix.py |
Removes the unused, unregistered handler. |
src/nodenorm/handlers/conflations.py |
Returns an object-wrapped conflation list and supports HEAD. |
src/nodenorm/handlers/base.py |
Adds shared JSON parsing with HTTP 400 errors. |
src/nodenorm/handlers/__init__.py |
Fixes route patterns and adds the OpenAPI redirect. |
src/nodenorm/config/config.default.json |
Excludes the OpenAPI redirect from tracing. |
README.md |
Documents version 1.2.0 and OpenAPI maintenance checks. |
pyproject.toml |
Adds OpenAPI and JSON Schema test dependencies. |
docker/configuration/config.json |
Applies the tracing exclusion to Docker deployments. |
Review effort: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2 tasks
gaurav
added a commit
to TranslatorSRI/babel-validation
that referenced
this pull request
Nov 7, 2026
#154 moved prod to NodeNorm ES, and prod still runs 1.0.0, from before biothings/NodeNormalizationAPI#58 fixed /get_allowed_conflations answering HTTP 500; ci and test run 1.2.0 and answer correctly. An xfail keyed on the backend alone would be wrong on one side or the other, so it is keyed on the version /status reports. That also covers the documented-paths test, which fails on prod for the same 500 and nothing else. A target whose /status has no x.y.z version fails rather than guesses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 7, 2026
gaurav
added a commit
to TranslatorSRI/babel-validation
that referenced
this pull request
Nov 9, 2026
…document (#153) NodeNorm Redis and NodeNorm ES are meant to be interchangeable for the endpoints they share, but three NodeNorm ES endpoints answered differently from Redis and from ES's own OpenAPI document: `GET /get_allowed_conflations` returned HTTP 500, `POST /get_setid` returns an object keyed `"0"`, `"1"`, … instead of a list, and `/get_semantic_types` lower-cases Biolink classes. This PR adds `tests/nodenorm/test_nodenorm_endpoints.py`, which runs against every target and would have caught all three. It also checks that each deployment documents the endpoints both backends share, and that every path its OpenAPI document lists is actually served. biothings/NodeNormalizationAPI#58, released as NodeNorm ES 1.2.0, fixed the conflations 500; `ci` and `test` run 1.2.0, prod still runs 1.0.0. The setid shape and the semantic-types casing are fixed by biothings/NodeNormalizationAPI#48 and #59, both still open. What is still wrong is a strict xfail on the ES targets, so an unexpected pass reports when a marker can come out. ## Calls worth overruling - **Each xfail passes `raises=KnownElasticsearchDiscrepancy`, and that class is raised only on the bug's own signature.** That means only lower-cased `biolink:` classes, only an object keyed `"0"`..`"n-1"` with a set ID under each key, and only Tornado's bare 500 page while `get_normalized_nodes` still works. Anything else the same check catches fails with a plain assert. A bare strict xfail, or a class raised for "not a list" or "any 500", would also absorb an outage or an unrelated regression. [`CLAUDE.md`](https://github.com/TranslatorSRI/babel-validation/blob/46e69ea/CLAUDE.md#testing-patterns) records the pattern, and why an imperative `pytest.xfail()` doesn't work with the dashboard. - **The conflations xfail is keyed on the version `/status` reports**, not just the backend: ES targets older than 1.2.0 expect the 500, later ones must pass. `nodenorm_version()` in `service_checks.py` reads `1.2.0`, `v1.2.1`, `1.3`, `1.3.0-rc1` and PEP 440's `1.3.0.dev0` or `1.2.0.post1`; a suffix is ignored, so a pre-release counts as its release. A target whose `/status` has no release number at all fails rather than guessing. - **The path sweep skips `/get_allowed_conflations` where its 500 is expected**, instead of being a second xfail for the same bug. `test_get_allowed_conflations` tracks it, and the sweep still checks every other path on prod. - **The shared-endpoint list is the five both backends serve**, not everything Redis documents. NodeNorm ES deliberately leaves out `/query`, `/asyncquery` and `/get_curie_prefixes` (biothings/NodeNormalizationAPI#36). - **A path counts as served unless a bare GET returns 404 or 5xx.** A POST-only or parameter-requiring path legitimately answers 405, 400 or 422. A timeout or refused connection is recorded against its path rather than raised, so one hung path doesn't hide the rest. - **The OpenAPI document is treated as untrusted.** Its path keys must be plain `/segment` paths, and a document may list at most 20, before any URL is built from them, so a key such as `//host/x` cannot send a request elsewhere. The sweep requests five paths at a time with a (5s, 10s) connect/read timeout, so even a hung deployment fails inside pytest-timeout's 300s with each path named. Every message goes through `truncated_repr()`. - **The setid test checks only the shape**, with a single POST. `test_setid_basic` already compares the set IDs from GET and POST. It now asserts the POST succeeded and returned a list, showing the status and body, rather than failing with a `TypeError`. It gets no xfail, because it also fails on Redis targets today (water now normalizes to `CHEBI:15377`), and an xfail would hide that. ## Also here - `service_checks.py` gains `get_openapi()`, which caches each target's OpenAPI document per process the way `get_status()` caches `/status`. Both `test_openapi_json` tests and the new tests share it. - The shared fetch checks for a JSON object inside the cache, so a brief 200 with `null` is fetched again by the next test rather than remembered. - Every request in `test_nodenorm_setid.py` now has a timeout. ## Verification On 46e69ea, against `prod`, `test`, `ci`, `ci-nameres-es`, `dev` and `exp`, every test in the new file passes or xfails as expected. The nine xfails are semantic types and setid on the four ES targets, plus conflations on prod. The unit suite passes. Outside the new file, the touched tests fail on exactly the targets they fail on `main`, for reasons in the deployments: missing or wrong `x-translator` blocks and a `maxEdges` schema error in `test_openapi_json`, non-JSON setid bodies on the ES targets, and water now normalizing to `CHEBI:15377` in `test_setid_basic`. Faked responses confirmed that an error object, a non-Biolink type, or a 500 during an outage or on 1.2.0 fails outright. ## After merging Nothing blocks merging. The markers come out as the strict XPASSes report: conflations once prod reaches 1.2.0, setid with biothings/NodeNormalizationAPI#48, and semantic types with biothings/NodeNormalizationAPI#59. Whether lower-cased Biolink classes would also be acceptable is a separate question, #158. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NodeNorm ES still publishes a copied OpenAPI document with stale service metadata, irrelevant TRAPI schemas, and endpoint contracts that do not match the Elasticsearch implementation. This updates the document for SmartAPI registration, corrects the affected endpoint responses and input handling, and exposes the document at
/openapi.json.Continues #42 with Gaurav Vaidya’s original commits for compatibility with the current
mainbranches:X-Babel-Versionheaders, and/statuscontract.The OpenAPI sync tests now resolve shared parameter references, recognize Tornado query readers, and check each HTTP method and its helpers separately. Offline response-schema checks cover both Babel releases and the current status response.
Validation: 290 tests passed, plus 12 subtests. Four integration tests requiring the
su10Elasticsearch host were excluded. OpenAPI file/served-document validation and diff whitespace checks pass.Closes #35. Closes #36. Closes #47. Supports the SmartAPI registration work in #34; registration follows production deployment.