Spec 005: live Reactome data and analysis

specs/005-mcp-live-data/checklists/requirements.md

@@ -0,0 +1,57 @@
+# Specification Quality Checklist: Live Reactome Data and Analysis
+
+**Purpose**: Validate specification completeness and quality before planning
+**Created**: 2026-09-10
+**Feature**: [spec.md](../spec.md)
+
+## Content Quality
+
+- [x] No implementation details beyond what the contributed PRs already fix
+- [x] Focused on user value and business needs
+- [x] Written for non-technical stakeholders
+- [x] All mandatory sections completed
+
+## Requirement Completeness
+
+- [x] No [NEEDS CLARIFICATION] markers remain
+- [x] Requirements are testable and unambiguous
+- [x] Success criteria are measurable
+- [x] All acceptance scenarios are defined
+- [x] Edge cases are identified
+- [x] Scope is clearly bounded
+- [x] Dependencies and assumptions identified
+
+## Adversarial review of this specification
+
+Every load-bearing claim was checked rather than reasoned about, after spec 004
+shipped a wrong one.
+
+| claim | how it was checked | verdict |
+|---|---|---|
+| "the bundle is two releases behind" | `cat embeddings/current` → Release95; `reactome.org/ContentService/data/database/version` → 97 | **verified** |
+| "it was already stale when built" | bundle mtime 2026-09-02, eight days ago | **verified** — so this is not drift from age |
+| "`reactome-mcp` is a prototype" | its own repository description: *"This is just a prototype for now"*, last push 2026-07-01 | **verified** |
+| "#127 spawns a subprocess" | `asyncio.create_subprocess_exec` with stdio pipes, in its diff | **verified** |
+| "#142 duplicates the existing router" | it adds `create_query_router`; `create_intent_classifier` already exists and routes reactome/userguide | **verified** |
+
+### The claim this specification does not make
+
+That MCP fixes staleness. It would, but so would rebuilding the bundle, and the
+rebuild needs no dependency, no subprocess and no router. Stating it that way is the
+whole contribution of this document: the PRs' own justification is mostly a cheaper
+problem wearing the expensive problem's clothes.
+
+### Not verified, and worth knowing
+
+Nobody has run `reactome-mcp` from this repository. Its 53 tools, their latency and
+their behaviour under failure are taken from #127's description, not observed. Any
+plan built on this should start by running it once.
+
+## Notes
+
+Deviations from the template, deliberate:
+
+1. **Three contributed PRs are reviewed in the spec body.** They are the starting
+   material and the reason to take or reject each is inseparable from reading them.
+2. **The spec argues against most of its own feature's justification.** That is the
+   finding, not a digression.

specs/005-mcp-live-data/spec.md

@@ -0,0 +1,203 @@
+# Feature Specification: Live Reactome Data and Analysis
+
+**Feature Branch**: `spec/mcp-live-data`
+
+**Created**: 2026-09-10
+
+**Status**: Draft. Three contributed PRs; two decisions (D1, D2) for the team.
+
+**Input**: #127, #137 and #142 from @GovindhKishore, integrating the `reactome-mcp`
+server so the chatbot can query live Reactome APIs and run enrichment analysis.
+
+## Two problems, wrongly bundled as one
+
+The PRs are motivated by a single sentence: MCP tools *"query Reactome APIs directly,
+meaning they always return current data regardless of when embeddings were built."*
+That is true, and it conflates two problems with very different costs.
+
+### Problem 1 — the answers are two releases out of date
+
+Measured today:
+
+| | |
+|---|---|
+| installed bundle | **Release95** |
+| bundle built | 2026-09-02, eight days ago |
+| `reactome.org/ContentService/data/database/version` | **97** |
+
+The bundle is two releases behind, and it was already two behind on the day it was
+built. So this is not drift from age — nothing rebuilt it against a current release.
+
+**This does not need MCP.** It needs the bundle rebuilt, and rebuilt on a schedule.
+That is `bin/embeddings_manager` and a cron entry, against a capability the
+repository already has.
+
+### Problem 2 — the chatbot cannot analyse anything
+
+`reactome-mcp` exposes enrichment analysis, pathway traversal and entity lookup.
+None of that is expressible as similarity search over stored text. No amount of
+rebuilding fixes it, because it is not a retrieval problem: the user gives a gene
+list and wants a computation.
+
+**This is the part only MCP can do**, and it is the honest reason to take the work.
+
+Separating them matters because Problem 1 is most of the stated benefit and the
+cheapest fix, while Problem 2 is the smaller-sounding benefit that actually requires
+the architecture.
+
+## What the three PRs do
+
+| PR | adds | lines |
+|---|---|---|
+| #127 | an MCP client speaking JSON-RPC over stdio to a spawned subprocess | +161 |
+| #137 | five MCP tools wrapped as LangChain `StructuredTool`s, wired to React-to-Me | +317/−20 |
+| #142 | an LLM router choosing between RAG, MCP search and MCP analysis | +450/−20 |
+
+They build on each other in order and are the work of one contributor. Taken
+together they are a coherent design, and the sequencing is right: client, then
+tools, then routing.
+
+## What taking them costs
+
+**A prototype dependency.** `reactome/reactome-mcp` is Reactome's own repository,
+which is the good case — but its description reads *"This is just a prototype for
+now"* and it was last pushed 2026-07-01, over two months ago. The chatbot would take
+a runtime dependency on it.
+
+**A subprocess in the container.** #127 uses `asyncio.create_subprocess_exec` with
+stdio pipes, so the image must carry the MCP server and its runtime, and each chat
+process spawns and supervises a child. The container now runs as a non-root user
+(#198), which that must work under. Nothing today spawns a process; this would be
+the first.
+
+**A second router.** #142 adds `create_query_router` alongside the existing
+`create_intent_classifier`, which already routes between `reactome` and `userguide`.
+Two LLM classification calls on the same question, in a pipeline where removing
+LLM calls was the point of spec 001 — retrieval went from 21 calls to one. The
+existing classifier should gain the new destinations rather than acquire a sibling.
+
+## User Scenarios & Testing *(mandatory)*
+
+### User Story 1 — Answers reflect the current release (Priority: P1)
+
+A user asks about a pathway added in Release 96 or 97 and gets it.
+
+**Why this priority**: It is the largest part of the stated benefit and by far the
+cheapest to deliver. It is P1 *and* it is not what the PRs build.
+
+**Independent Test**: Rebuild the bundle against Release97, ask about content added
+since 95, compare against today.
+
+**Acceptance Scenarios**:
+
+1. **Given** a rebuilt bundle, **When** a user asks about recent content, **Then**
+   the answer includes it.
+2. **Given** a release cadence, **When** a new release ships, **Then** rebuilding is
+   a scheduled operation rather than a thing someone remembers.
+
+---
+
+### User Story 2 — The chatbot can run an enrichment analysis (Priority: P2)
+
+A user pastes a gene list and asks which pathways are over-represented. The chatbot
+runs the analysis and explains the result.
+
+**Why this priority**: Genuinely new capability, and impossible without something
+like MCP. P2 below staleness only because staleness affects every question and this
+affects a class of question we do not serve at all today.
+
+**Acceptance Scenarios**:
+
+1. **Given** a gene list, **When** analysis is requested, **Then** a real Reactome
+   analysis runs and its result is explained.
+2. **Given** the MCP server is unavailable, **When** analysis is requested, **Then**
+   the chatbot says so plainly rather than answering from the vector store as if it
+   had analysed anything.
+
+---
+
+### User Story 3 — Routing costs one classification, not two (Priority: P2)
+
+A question is classified once, into one of the destinations available.
+
+**Acceptance Scenarios**:
+
+1. **Given** MCP is enabled, **When** a question arrives, **Then** exactly one
+   classification call is made.
+2. **Given** MCP is disabled, **When** a question arrives, **Then** behaviour and
+   call count are exactly as today.
+
+### Edge Cases
+
+- **The MCP server dies mid-conversation.** A supervised subprocess needs a defined
+  answer: restart, degrade to RAG, or fail. Silently degrading to RAG is the worst
+  option, because an analysis question would get a retrieval answer.
+- **The prototype changes its tool surface.** Five tools are wrapped by name; a
+  rename upstream breaks them at call time, not at start-up.
+- **Live and stored data disagree.** The vector store says Release95, the API says
+  97. An answer that mixes both without saying so is a new class of wrong.
+- **Analysis latency.** Enrichment is not a sub-second call, and the chat surface
+  already runs at 22s per question.
+
+## Requirements *(mandatory)*
+
+### Functional Requirements
+
+- **FR-001**: The installed bundle MUST be rebuildable against the current release
+  without code changes.
+- **FR-002**: Rebuilding MUST be schedulable, not a remembered manual step.
+- **FR-003**: The deployed release version MUST be visible — an operator must be able
+  to see which release is being answered from.
+- **FR-004**: If MCP is adopted, it MUST be optional: with it disabled, behaviour and
+  LLM call count are exactly as today.
+- **FR-005**: If MCP is adopted, questions MUST be classified exactly once, by
+  extending the existing intent classifier rather than adding a second.
+- **FR-006**: An unavailable MCP server MUST produce an explicit failure for
+  analysis questions, never a silent fall back to retrieval.
+- **FR-007**: Where an answer draws on live API data rather than the bundle, that
+  MUST be distinguishable.
+
+## Success Criteria *(mandatory)*
+
+- **SC-001**: The gap between the deployed bundle's release and Reactome's current
+  release is at most one.
+- **SC-002**: An operator can determine the answering release without reading code.
+- **SC-003**: With MCP disabled, a question costs the same LLM calls as today.
+- **SC-004**: A gene-list question produces a real analysis or an explicit refusal,
+  never a retrieval answer dressed as one.
+
+## Decisions for the team
+
+### D1 — Rebuild the bundle now, independently of MCP?
+
+Recommended: **yes, and first.** Two releases behind is the larger share of the
+stated benefit, it needs no new dependency, no subprocess and no router, and it can
+ship this week. It also makes the MCP decision honest by removing staleness from its
+justification, leaving analysis — which is the real case.
+
+### D2 — Adopt MCP for analysis?
+
+| option | what it means |
+|---|---|
+| **A. Not yet** | Rebuild bundles, revisit when `reactome-mcp` is past prototype. Costs nothing; the capability gap remains. |
+| **B. Adopt behind a flag** | Take #127 and #137, fold routing into the existing classifier rather than #142's second one, ship disabled by default. Real capability, contained blast radius, a prototype dependency that cannot affect anyone who has not enabled it. |
+| **C. Adopt fully** | Everything the PRs propose, on by default. Fastest to the capability, and puts a self-described prototype and a supervised subprocess in the path of every user. |
+
+**Recommendation: B**, contingent on D1 being done first. It buys the irreplaceable
+part while the dependency is still a prototype, and FR-004 means the cost of being
+wrong is a flag nobody turned on.
+
+## Assumptions
+
+- `reactome-mcp` staying a Reactome project. If it were third-party the answer would
+  be different; a prototype from the same organisation is a shared risk, not an
+  external one.
+- Rebuilding bundles is routine. If it is not, that is the finding, and it makes
+  D1 more urgent rather than less.
+
+## Out of Scope
+
+- Which MCP tools to wrap beyond the five #137 chose.
+- Replacing retrieval with MCP. Retrieval over curated text is what the product is;
+  MCP adds computation beside it.
+- Analysis result presentation in the UI.