Spec 010: plan, research and 24 tasks — build the endpoint slow, unblock the website repo

specs/010-search-page-answers/plan.md

@@ -0,0 +1,108 @@
+# Implementation Plan: Chatbot Answers in the Website Search Results
+
+**Branch**: `010-search-page-answers` | **Date**: 2026-09-17 | **Spec**: [spec.md](./spec.md)
+
+**Input**: Feature specification from `/specs/010-search-page-answers/spec.md`
+
+## Summary
+
+Build the endpoint first, slow, and let the website integrate against something real
+while the latency work happens behind it. Their side is blocked on nothing but this
+service's absence; ours is blocked on nothing but time.
+
+Three things do not exist: an HTTP answer endpoint, any streaming surface, and
+token verification. The latency budget (FR-005) is deliberately **not** a gate on the
+first increment -- an endpoint that answers correctly in fifteen seconds unblocks the
+other repo; an endpoint that does not exist blocks it completely.
+
+## Technical Context
+
+**Language/Version**: Python 3.12
+
+**Primary Dependencies**: FastAPI, langchain 1.x, langgraph, Chainlit (mounted on the
+same app)
+
+**Storage**: none new. The graph, bundle and MCP are as they are
+
+**Testing**: pytest; `bin/answer-sweep` for answer equivalence
+
+**Target Platform**: the existing `biochat_beta_guest` container behind nginx
+
+**Project Type**: single project
+
+**Performance Goals**: FR-005 is first token ≤2s, complete ≤10s. Today p50 is 15.2s,
+p90 22.4s, with no first-token concept at all
+
+**Constraints**: the endpoint and the chat UI must share one graph (SC-003); a
+failure must render as "no panel", never a broken one (FR-006)
+
+**Scale/Scope**: one endpoint, one streaming surface, one verification step
+
+## Constitution Check
+
+| Principle | Gate | How this plan satisfies it |
+|---|---|---|
+| I. Verify the path a user takes | Test through the served path | The endpoint is tested over HTTP with a real client, not by calling the handler. The served path is async and mounted alongside Chainlit, which is where mounting order and middleware interact |
+| II. Measure retrieval changes | Before-and-after on real questions | This adds no retrieval change. If the latency work later touches retrieval, that measurement belongs to it, not here |
+| III. Characterization tests | Pin current behaviour first | The captcha middleware currently intercepts every path under `CHAINLIT_URI`. A test pins that before a new route is added beside it |
+| IV. Fail loudly | No plausible substitutes | An unverified request is refused, not answered anonymously. A missing signing key stops the endpoint from starting rather than accepting everything |
+| V. Source of truth | No hand-synchronised constants | Citations come from retrieved documents' `st_id` metadata, not from parsing anchors out of the model's prose |
+
+**Gate result**: pass.
+
+### The one that needs care
+
+Principle IV cuts both ways here. FR-006 says fail **invisibly** -- any error renders
+as "no panel" -- and Principle IV says fail **loudly**. They are not in conflict but
+the line matters: *misconfiguration* stops the process (no signing key, no bundle),
+while a *runtime* failure answering one question returns `state: failed` and is
+logged. The first is loud because an operator must fix it; the second is quiet
+because a search page must not break.
+
+## Key design decisions, from reading the code
+
+**Citations come from retrieval, not from the prose.** The prompt currently instructs
+the model to emit `<a href="...">` anchors inline. Parsing those back out of a token
+stream would be fragile and would couple the contract to prompt wording. The
+retrieved documents already carry `st_id` in metadata, so citation events are emitted
+from the retrieval result. This is why the contract specifies stable IDs rather than
+HTML.
+
+**Streaming needs a surface that does not exist.** `AgentGraph` exposes `ainvoke` and
+nothing else; Chainlit gets its streaming from `AsyncLangchainCallbackHandler` with
+`final_stream`, not from a graph-level API. Two options, resolved in research.md.
+
+**The FastAPI app does not hold a graph.** `AgentGraph` is constructed in
+`bin/chat-chainlit.py`, which Chainlit mounts. The endpoint needs one, and building a
+second would double 51.5s of startup and risk the two answering differently, which
+SC-003 forbids.
+
+## Project Structure
+
+```
+specs/010-search-page-answers/
+├── spec.md, plan.md, research.md
+├── contracts/answer_endpoint.md
+├── quickstart.md
+└── tasks.md
+
+src/
+├── api/                      # new: the endpoint, its models, SSE framing
+├── agent/graph.py            # gains a streaming surface
+└── util/human_token.py       # new: signature verification only
+
+bin/chat-fastapi.py           # mounts the router; middleware ordering matters
+tests/api/                    # new
+```
+
+## Complexity Tracking
+
+| Addition | Current need | Why the simpler option is insufficient |
+|---|---|---|
+| A second answer surface beside Chainlit | The search page cannot use a websocket chat UI | Chainlit's socket protocol is not a documented API and is not something another repo should couple to |
+| Streaming on `AgentGraph` | FR-002, and 10s is only survivable if text appears early | Returning a complete answer means a blank panel for fifteen seconds |
+
+## Phases
+
+Phase 0 research: [research.md](./research.md). Phase 1 design: the contract already
+exists and was agreed with the website session; `quickstart.md` covers validation.

specs/010-search-page-answers/quickstart.md

@@ -0,0 +1,46 @@
+# Quickstart: validating the answer endpoint
+
+## Baseline, before any of this is built
+
+Measured 2026-09-17 across the 15 tracked sweep questions, so Phase 5 has a before:
+
+- p50 **15.2s**, p90 **22.4s**, min 6.7s, max 31.5s
+- no first-token concept: the answer arrives whole or not at all
+
+## Once the endpoint exists
+
+```bash
+curl -N -X POST https://beta.reactome.org/chat/api/answer \
+  -H 'Content-Type: application/json' \
+  -d '{"question":"what does CDK5 phosphorylate in Alzheimer disease?",
+       "human_token":"<token>"}'
+```
+
+`-N` matters. Without it curl buffers and the stream looks like a single slow
+response, which is the thing being tested.
+
+Expect `event: start` immediately, `event: token` repeatedly, `event: citation` as
+retrieval resolves, and `event: done` with a state.
+
+## What to check, and what not to conclude
+
+**Tokens actually stream.** Not that the endpoint returns 200. A 200 that completes
+without streaming is a pass on the wrong question -- the same shape as the userguide
+shell that returned 200 and contained stylesheets. Count the events.
+
+**A refused request makes no model call.** Assert on a patched graph, not on how fast
+the refusal came back; a fast refusal and an expensive one look alike from outside.
+
+**The endpoint and the chat agree.** Same question through `bin/answer-sweep` and
+through the endpoint. Two surfaces that can disagree about the same question is a
+defect, not a feature (SC-003).
+
+**Failure renders as no panel.** Stop the graph, or send a malformed question, and
+confirm a `done` with a non-`answered` state rather than a hang or a 500 body the
+page would try to render.
+
+## What this does not prove
+
+Latency. The first increment is deliberately slow -- correct, verified and streaming,
+at roughly today's p50. FR-005's 2s/10s is Phase 5, and the website repo is not
+waiting on it.

specs/010-search-page-answers/research.md

@@ -0,0 +1,84 @@
+# Phase 0 Research: the answer endpoint
+
+## R1. How does the endpoint stream, given `AgentGraph` has no streaming surface?
+
+`AgentGraph` exposes `ainvoke` and nothing else. Chainlit's streaming comes from
+`AsyncLangchainCallbackHandler` with `final_stream` -- a Chainlit object, not
+something a plain HTTP handler can use.
+
+| Option | How | Verdict |
+|---|---|---|
+| A. `astream_events` on the compiled graph | LangGraph's own event stream, filtered to the final LLM's tokens | **Chosen.** It is the framework's supported streaming API, and it exposes retrieval events too, which is where citations come from |
+| B. A callback handler pushing to an `asyncio.Queue` | Mirrors what Chainlit does; the SSE response drains the queue | Works, and is what to fall back to if filtering A's event stream proves unreliable. Costs a bespoke handler and a lifetime to manage |
+| C. Two graphs, one per surface | Simplest to write | Rejected. Doubles 51.5s of startup and lets the panel and the chat disagree, which SC-003 forbids |
+
+**Decision: A, with B as the named fallback.** The risk in A is that the event
+filtering depends on which node emits the final answer, so a graph change could
+silently stop the stream. A test asserting that tokens actually arrive -- not merely
+that the endpoint returns 200 -- is the guard, and that lesson is fresh: a 200 with
+no content is exactly how the userguide shell fooled two sessions today.
+
+## R2. Where does the graph live?
+
+`bin/chat-chainlit.py` constructs `AgentGraph` at module scope; Chainlit is mounted
+onto the FastAPI app in `bin/chat-fastapi.py`. The endpoint needs the *same*
+instance, not another one.
+
+**Decision**: construct it once in the FastAPI app's lifespan and let both surfaces
+use it. Failing that, a module-level accessor the Chainlit app also imports.
+
+**Rationale**: SC-003 requires the endpoint and the chat to answer identically, and
+the cheapest way to guarantee that is one object. A second graph also pays 51.5s of
+startup again and doubles the memory of the BM25 indexes.
+
+## R3. How are citations produced?
+
+Not by parsing the model's prose. The answer prompt instructs `<a href=...>` anchors,
+and pulling those out of a token stream would couple the wire contract to prompt
+wording -- prompt changes have happened twice this week.
+
+**Decision**: emit citation events from the retrieved documents, whose metadata
+already carries `st_id`. Deduplicate, and emit as each document set arrives.
+
+**Consequence worth stating**: the citations are then "what retrieval found", which
+is a superset of "what the answer used". The contract does not promise otherwise, and
+a panel showing sources the prose did not cite is better than a panel that mis-parses
+prose. If exactness is wanted later it needs the model to emit IDs, which is a prompt
+change with its own measurement.
+
+## R4. How is the human token verified?
+
+Agreed with the website session on 2026-09-17: they mint after their own captcha,
+asymmetric signing, they hold the signing key and this service holds only a verifying
+key. The query budget is theirs, enforced before the call, because they proxy every
+request.
+
+**Decision here**: verification is signature + expiry, stateless, and nothing else.
+No consumption tracking, no shared secret, no second counter that can disagree.
+
+**Startup**: a missing or unreadable verifying key stops the process. Principle IV --
+an endpoint that accepts everything because its key is absent is the worst outcome
+available, and it would test clean.
+
+## R5. Does the new route collide with the captcha middleware?
+
+`verify_captcha_middleware` intercepts every path and returns early only for a small
+allowlist under `CHAINLIT_URI`. A new route under `/chat/api/` would otherwise be
+redirected to the captcha page.
+
+**Decision**: the middleware must let the API path through, and the endpoint does its
+own verification. A characterization test pins the middleware's current behaviour
+first, so this change is deliberate rather than incidental.
+
+This is also a reminder that the middleware is where a bug was found today: it read
+`os.environ` for a secret that comes from `get_secret`, so a mounted Docker secret
+silently disabled the captcha. Adding a route beside it warrants care.
+
+## R6. What is the first increment?
+
+An endpoint that answers correctly and slowly, streaming, with verification. Not fast.
+
+**Rationale**: the website session cannot build the panel or the token handshake
+against nothing, and is otherwise idle on this. p50 15.2s is too slow to ship to
+users and entirely good enough to integrate against. The latency work is real and
+separate, and spec 009 plus query-expansion reduction are its two known levers.

specs/010-search-page-answers/tasks.md

@@ -0,0 +1,76 @@
+# Tasks: Chatbot Answers in the Website Search Results
+
+**Feature**: 010-search-page-answers | **Plan**: [plan.md](./plan.md) | **Spec**: [spec.md](./spec.md)
+
+Test tasks included: this adds a public surface with an authentication boundary, and
+Principle III wants current behaviour pinned before a route is added beside the
+captcha middleware.
+
+**MVP is Phase 3.** An endpoint that answers correctly and slowly unblocks the
+website repo entirely. Speed is Phase 5 and does not gate the handover.
+
+## Phase 1: Setup
+
+- [ ] T001 Create branch `010-search-page-answers`; confirm `.specify/feature.json` points at specs/010-search-page-answers
+- [ ] T002 [P] Record today's baseline in specs/010-search-page-answers/quickstart.md: p50 15.2s, p90 22.4s over the 15 sweep questions, so Phase 5 has a before
+
+## Phase 2: Foundational (blocks the endpoint)
+
+- [ ] T003 Pin the captcha middleware's current behaviour in tests/api/test_middleware_scope.py: every path under CHAINLIT_URI redirects to the captcha page except the existing allowlist
+- [ ] T004 Give `AgentGraph` one home the FastAPI app can reach, in bin/chat-fastapi.py via lifespan, so the endpoint and Chainlit share one instance (SC-003) rather than paying 51.5s of startup twice
+- [ ] T005 Add a streaming surface to src/agent/graph.py using the compiled graph's `astream_events`, yielding answer tokens and retrieved documents
+- [ ] T006 Test that T005 yields more than one token for a real question — not that it returns 200. A surface that completes without streaming is the failure this must catch
+
+## Phase 3: User Story 1 — a verified person sees an answer forming (P1)
+
+**Independent test**: post a question with a valid token; assert the first event
+arrives, tokens stream, citations resolve to real stable IDs, and `done` carries a
+state.
+
+- [ ] T007 [P] [US1] Add src/util/human_token.py: verify signature and expiry only, stateless, no consumption tracking
+- [ ] T008 [P] [US1] Test human_token in tests/util/test_human_token.py: valid, expired, wrong key, tampered payload, absent — every failure refuses
+- [ ] T009 [US1] Refuse at startup in src/util/human_token.py when the verifying key is missing or unreadable, rather than accepting everything (Principle IV)
+- [ ] T010 [US1] Add the SSE endpoint in src/api/answer.py implementing contracts/answer_endpoint.md: start, token, citation, done
+- [ ] T011 [US1] Emit citations from retrieved documents' `st_id` metadata, deduplicated — never by parsing anchors out of the model's prose
+- [ ] T012 [US1] Mount the router in bin/chat-fastapi.py and let the captcha middleware pass /chat/api/ through, since the endpoint verifies its own caller
+- [ ] T013 [US1] Test over HTTP with a real client in tests/api/test_answer_endpoint.py, not by calling the handler — mounting order and middleware only interact in the served path (Principle I)
+- [ ] T014 [US1] Assert the answer matches the chat UI's for the same question (SC-003); two surfaces that can disagree is a defect
+
+## Phase 4: User Story 2 — no answer without a verified person (P1)
+
+- [ ] T015 [US2] Refuse missing, expired, malformed and wrongly-signed tokens before any model call, in src/api/answer.py
+- [ ] T016 [US2] Test that no model call happens for a refused request in tests/api/test_answer_endpoint.py, by asserting on a patched graph rather than on timing (SC-002)
+- [ ] T017 [P] [US2] Rate limit per token as a backstop; the budget is the website's, enforced before the call reaches here (FR-008)
+- [ ] T018 [US2] Return `state: failed` with no partial answer on any internal error, so the page renders no panel (FR-006)
+
+## Phase 5: Latency (does NOT gate the handover)
+
+- [ ] T019 Measure first-token and completion separately across the tracked questions; publish the distribution, not one question
+- [ ] T020 Reduce query expansion from 5 variants, measuring recall with bin/retrieval_baseline — 2.4s and a 5x retrieval fan-out, the larger of the two known levers
+- [ ] T021 Land spec 009 collection routing and re-measure
+- [ ] T022 Re-assess FR-005 against the result and say plainly whether 2s/10s is reachable
+
+## Phase 6: Handover
+
+- [ ] T023 Tell the website session the endpoint exists, with a curl that streams, and what it does not yet do
+- [ ] T024 [P] Update specs/010-search-page-answers/spec.md status and record the measured first-token and completion times
+
+## Dependencies
+
+```
+Phase 1 -> Phase 2 -> Phase 3 -> Phase 4 -> Phase 6
+                                   Phase 5 runs alongside 4 and 6
+```
+
+T004 blocks T005; T005 blocks T010; T007 blocks T012; T010 blocks T013, T014, T015.
+
+## Parallel opportunities
+
+- T007 and T008 alongside T004-T006: token verification touches nothing the graph does
+- T017 and T024: different files
+
+## Implementation strategy
+
+Ship Phases 1-4 as one PR: a correct, slow, verified, streaming endpoint. That is the
+handover. Phase 5 follows on its own, with its own measurements, and the website repo
+is not waiting on it.