Serve the species lists from node - #289
Merged
Merged
Conversation
Two more ContentService endpoints on the paths Java already uses, which is the point: to a caller there is one ContentService, and which process answers is not their concern. `main` is not a property. All 96 Species nodes carry the same six, and the 16 "main" ones are the ones a TopLevelPathway points at -- checked against the graph rather than inferred from 16 matching 16, because two numbers agreeing is not a rule. The two differ in **order** as well as contents, which is the part a port loses. `main` pins Homo sapiens first and sorts the rest by name; `all` is plain alphabetical with human in its place. Reactome pins human because it is the reference species everything else is inferred from, and a selector that buries it between Gallus and Mus asks every reader to hunt for the common case. Both orders read off the live responses, not assumed. Written as two rows of one generator rather than one handler taking a flag, because a shared sort is how one of them silently acquires the other's order and the page still renders. `diff.mjs` reports 12 of 12 identical, the two new ones with no declared differences. That claim is worth what the check is worth, so it was tested: making `all` keep main's pinned order produces 190 differences, and the run before that one passed only because the service under test was still holding the unmutated code from before the edit. The unit test deliberately does not assert the ordering. It cannot -- that needs a database and a running Java service -- and the version that tried, by matching the handler's source text for "TopLevelPathway", would have passed on any wrong query that happened to mention it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Prompted by a reminder to check query cost whenever porting, and the answer was not in the query. `/data/database/version` answered in 286ms where Java took 1.1ms, because Java holds the value and this went to the graph every time. It is the most-called endpoint of the set: the site asks for the release on every page load, since it keys the bucket paths for diagrams, figures and icons. So the first endpoint ever ported -- chosen as the smallest possible one, to prove the plumbing -- was also the slowest, and nobody had measured it against the thing it replaced. `/data/database/name` was the same at 290ms. Both now use the same `cached` wrapper the lists use, which is safe for the reason Java's cache is: these change when the database is replaced, and that restarts the service. 286ms to 8.5ms on the first request and 1.4ms after, against Java's 3.2ms. Not an index problem, which was the first thing checked -- one node matched by label, where no index applies. A standalone script reports ~700ms for that same query while the running service answers it in 8.5ms, same credentials and same localhost instance; that is recorded in the comment as unexplained rather than dressed up, because the fix is the same either way and the number that matters is the service's. The species lists this sits beside were already cached, which is why they answer in 1.3ms against Java's 682ms. `diff.mjs` still reports 12 of 12 identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rything
I had recorded this as unexplained. It is explainable, and the explanation is a
rule worth having before porting anything else.
`MATCH (n:DBInfo) RETURN n.releaseNumber` matches exactly one node and takes
~700ms. The plan says why:
AllNodesScan + Filter n:DBInfo
There is **no token (LOOKUP) index** on this instance -- 36 indexes, none of
them one -- so a label match cannot use a label index and the planner reads
every node. There are 2,958,129 of them. The same value fetched through an
indexed property takes 29ms.
That is why the cost does not vary with how many nodes match or how many rows
return: one node and four hundred both scan three million. It is also why
Java's own /data/species/main takes 682ms.
Two rules for the next port, written next to `read` where a query gets written:
match an indexed property under the label the index is on; and where an
endpoint genuinely needs every node of a label -- the species lists do, and no
property expresses that -- accept the scan and `cached()` it, which the service
warms at startup so no reader waits for it.
The real fix is one statement from whoever owns the database:
CREATE LOOKUP INDEX node_labels FOR (n) ON EACH labels(n)
It would speed up the Java service by the same amount. Not taken here: it is a
schema write on an instance beta depends on.
Also corrects the previous commit's claim that caching took the version
endpoint to "8.5ms on the first request". It did not: the service warms cached
handlers at startup, so that 8.5ms was a warm read and the build had already
happened. The endpoint is ~1.4ms warm against Java's 3.2ms, and the scan is
paid once at boot.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The review found that caching this was a regression, not just a speed-up. `cached` held a value for the life of the process, justified as "a release restarts the service, and these lists only change with the graph". True of Java, whose WAR is redeployed. Not true of this: it runs as a container with `restart: unless-stopped`, and nothing in the release procedure or in any script on the host restarts it. I checked, after caching something where being stale is worse than being slow. The release number is exactly that. It keys the bucket paths for diagrams, figures and icons, so serving last release's number sends all of those requests to the wrong prefix -- and it would keep doing so until somebody noticed and restarted a container. Before this branch the endpoint was uncached and therefore always right; I would have traded correct-and-slow for fast-and -eventually-wrong. So `cached` takes an optional `ttlMs`, and the two database endpoints take 60 seconds: a minute of staleness after a release instead of forever, and still one scan a minute rather than one per request. Warmed at startup as before, so no reader waits for the first. The lists keep the unbounded form, and the reasoning is now written down rather than asserted: they are expensive to rebuild, only read by pages that render them, and a stale entry is a missing person or DOI until the next deploy rather than a wrong URL everywhere. A judgement about consequence, which the next person can disagree with knowingly. The test asserts the bound rather than the number, and fails on the version as this branch first wrote it: `expected Infinity to be less than 300000`. `diff.mjs` still reports 12 of 12 identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Decided: updating the database includes restarting this service. So the caches
go back to living for the life of the process, and the TTL added in the
previous commit is gone -- it was working around the absence of a rule, and a
rule is the better answer.
What changed is that it is now a rule rather than an inference. The comment used
to say "a release restarts the service", which was true of Java, whose WAR is
redeployed, and untrue of this, a container with `restart: unless-stopped` that
nothing restarted. That is the kind of sentence that is right until the day it
matters.
Because the rule depends on a person doing something, the cost of forgetting is
now observable. `/health` reports the release this process is serving. The
release number keys the bucket paths for diagrams, figures and icons, so a
process holding the previous one sends all of those to the wrong prefix while
every response still looks correct -- the failure has no other symptom. One
curl now shows it.
It reads that through the same cache the endpoint uses, deliberately. A health
check that queried the graph directly would report the right number while every
other response served the wrong one, which is worse than not reporting it: it
would confirm the thing that is broken.
Verified against a real graph -- `{"ok":true,"graph":true,"release":"97"}` --
and against no graph at all, where it still answers 200, because the process
being up is what it is asked about.
diff.mjs: 12 of 12 identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
adamjohnwright
added a commit
that referenced
this pull request
Sep 22, 2026
Replaces the staging directory from the previous commit with the standard
answer, which is also the one this compose file already uses three times over:
`render`, `content-node` and `mcp` all bake their code into an image. nginx was
the odd one out, reading its configuration from whatever was checked out.
What that cost: a feature branch's /mcp route was found sitting inside the
production-serving container, inert only because nginx reads its configuration
at start rather than continuously. The staged-directory fix worked, and was
something a newcomer had to discover. An image is a thing they already
understand, and "which configuration is live" becomes readable off the image
rather than inferred from a working tree.
Gone with it: scripts/stage-nginx.sh, the .staged directory and its gitignore
entry. The validation it did is now a documented step against the built image,
which exercises the certificate mounts that a build-time `nginx -t` cannot see:
docker compose build nginx
docker compose run --rm nginx nginx -t
docker compose up -d nginx
Certificates stay as runtime mounts. They are host secrets and nothing that
cannot be rebuilt from this repository belongs in the image.
Verified after deploying: the site answers 200, `/mcp` initialises, the Java
exporter renders, `/RenderService` renders, and the container has no
configuration mounts left at all.
**And it caught a live regression that was not mine.** `/ContentService/data/
species/main` was answering 404 on beta: #289 added the nginx route to node and
merged, but content-node was still running the image built before it -- ten
endpoints where main has twelve -- so a path Java used to answer had been
turned into a 404. Rebuilt and redeployed; all four node-served endpoints now
answer 200 and species/main leads with Homo sapiens.
That is the third time today a merge sat undeployed. The nginx half now cannot
drift, because the image is built from the repository; the services behind it
still can.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Two more ContentService endpoints, on the paths Java already uses — which is the point: to a caller there is one ContentService, and which process answers is not their concern.
What "main" actually is
Not a property. All 96
Speciesnodes carry the same six properties; the 16 "main" ones are the ones aTopLevelPathwaypoints at. Checked against the graph rather than inferred from 16 matching 16 — two numbers agreeing is not a rule.The two differ in order, not just contents
/data/species/main/data/species/allReactome pins human because it is the reference species everything else is inferred from; a selector that buries it between Gallus and Mus asks every reader to hunt for the common case.
Written as two rows of one generator rather than one handler taking a flag — a shared sort is how one of them silently acquires the other's order, and the page still renders.
Verification
diff.mjsreports 12 of 12 identical, the two new ones with no declared differences.That claim is worth what the check is worth, so I tested the check: making
allkeep main's pinned order produces 190 differences. The run before that one passed only because the service under test was still holding the unmutated code from before the edit — a vacuous pass I caught and redid.The unit test deliberately does not assert the ordering. It cannot — that needs a database and a running Java service — and the version that tried, by matching the handler's source text for
TopLevelPathway, would have passed on any wrong query that happened to mention it.Wiring
Exact contexts in both
proxy.conf.jsand nginx, not a/data/speciesprefix: Java serves other shapes under that prefix which node does not, and a prefix would claim them and 404 what works today.nginx -tpasses for dev, production and release.🤖 Generated with Claude Code