Skip to content

Update OpenAPI for SmartAPI registration and preserve multi-Babel behavior - #58

Merged
shuchenliu merged 26 commits into
mainfrom
pr42-openapi-compat
Nov 2, 2026
Merged

shuchenliu merged 26 commits into
mainfrom
pr42-openapi-compat

Conversation

@shuchenliu

@shuchenliu shuchenliu commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

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 main branches:

  • preserves multi-Babel release selection and X-Babel-Version headers, and
  • retains the current /status contract.
  • The API version advances to 1.2.0.

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 su10 Elasticsearch 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.

gaurav and others added 25 commits October 12, 2026 00:41
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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread src/nodenorm/webapp/openapi.json Resolved
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants