From c036dd6a8c7bdf262352e40a594dbda99b3e6f3b Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 25 Sep 2026 16:17:38 +0000 Subject: [PATCH] Continue in chat: hand an analysis summary into a new chat tab Spec 013, Story 1. Beside an analysis summary on the website, "Continue in chat" opens the chat in a new tab with that summary already the thread's previous turn, so the reader's first message can be a follow-up. **The handoff travels in the URL fragment and is bound to the tab.** The website mints an ID with `POST /api/handoff` and opens `/chat/guest/#handoff=`. Browsers never send a fragment to a server, so the ID stays out of nginx logs and Referer headers. Chainlit 2.11 gives the app no page URL on connect -- only cookies -- and a cookie is shared by every tab, so two handoffs opened together could seed the wrong one. Instead a block in `public/custom.js` reads the fragment and posts it; Chainlit forwards every window message to `@cl.on_window_message` over that tab's own socket. **The retry is load-bearing, measured.** A single post in the first second after load is lost 15 times out of 15, because Chainlit drops window messages until its socket is up; from two seconds on it is claimed 6 of 6. Posting once on load -- the obvious implementation -- would never have worked. The script retries every 500 ms for up to 20 s and stops on its own acknowledgement, and since a claim can therefore arrive twice, redemption is idempotent per session. **The chat continues the summary the reader saw; it does not regenerate one** (FR-002). Generation here is not reproducible (~0.33 similarity run to run), so a handoff copies the summary text when it is minted. It can only be minted for a summary that already exists in the summary cache at the requested tier -- which is also how it refuses to widen disclosure (FR-003): an identifiers-tier handoff exists only if the reader chose identifiers on the website. **The model gets the data, not just the words.** The seeded turn is `[HumanMessage, AIMessage]`, the shape of every real turn, and the AI turn carries the summary plus the allow-listed analysis data rebuilt at the handoff's tier with the same functions the summary endpoint uses. Recorded as having ended at `postprocess`, the last node of all three profiles, so the next message is an ordinary turn with that history. Verified end to end in a browser against a real analysis on beta: the tab opens on the same text the summary endpoint produced; asked which pathway has the lowest FDR, the chat names the analysis's real top pathway with its exact FDR (4.2e-15). The control -- the same question with no handoff -- answers that the user guide does not cover it. An unknown ID says so rather than opening an empty chat. Adversarial review found: - **A promise the architecture could not keep.** The first endpoint also returned a `personal_path`. The store is in memory and the guest and logged-in chats are separate processes, so that link could only ever open on "couldn't load the summary". It now returns one `path`, for the chat served by the process that minted it; the spec records what Story 3 needs. - `current_release()` and `fetch_not_found()` can both return None, which the first version assumed they could not -- found by mypy, handled explicitly. - A claim arriving before `on_chat_start` would have seeded a thread named "None"; it falls back to the session id. Seeding failures now say so to the reader instead of leaving a chat that silently lacks the context, and claims are logged either side. Sabotage: fetching identifiers regardless of tier fails the aggregate test; looking the summary up at `aggregate` whatever was asked fails the cannot-widen test. Co-Authored-By: Claude Opus 5.5 --- bin/chat-chainlit.py | 70 +++++++++ bin/chat-fastapi.py | 2 + public/custom.js | 38 +++++ specs/013-continue-chat-hand/spec.md | 167 ++++++++++++++++++++ src/agent/graph.py | 24 +++ src/api/analysis_summary.py | 12 +- src/api/handoff.py | 137 +++++++++++++++++ src/handoff/__init__.py | 0 src/handoff/seed.py | 95 ++++++++++++ src/handoff/store.py | 89 +++++++++++ src/handoff/window.py | 57 +++++++ tests/api/test_handoff_endpoint.py | 219 +++++++++++++++++++++++++++ tests/handoff/test_handoff_seed.py | 139 +++++++++++++++++ tests/handoff/test_handoff_window.py | 64 ++++++++ 14 files changed, 1112 insertions(+), 1 deletion(-) create mode 100644 specs/013-continue-chat-hand/spec.md create mode 100644 src/api/handoff.py create mode 100644 src/handoff/__init__.py create mode 100644 src/handoff/seed.py create mode 100644 src/handoff/store.py create mode 100644 src/handoff/window.py create mode 100644 tests/api/test_handoff_endpoint.py create mode 100644 tests/handoff/test_handoff_seed.py create mode 100644 tests/handoff/test_handoff_window.py diff --git a/bin/chat-chainlit.py b/bin/chat-chainlit.py index ff714cac..8211f0bd 100644 --- a/bin/chat-chainlit.py +++ b/bin/chat-chainlit.py @@ -23,6 +23,9 @@ result_file_kwargs, run_analysis, ) +from handoff import seed +from handoff.store import handoffs +from handoff.window import acknowledgement, claimed_id from util.chainlit_helpers import ( PrefixedS3StorageClient, is_feature_enabled, @@ -42,6 +45,8 @@ mounted_secrets, ) +logger = logging.getLogger(__name__) + load_dotenv() # Before anything reads os.environ. Docker secrets, where mounted, take # precedence over .env; where not mounted, nothing changes. @@ -204,6 +209,71 @@ async def send_file(path: Path) -> None: await progress.remove() +async def continue_from_handoff(handoff_id: str) -> None: + """Start this thread from the summary the reader clicked from. + + The model gets the summary as its own previous turn plus the data it was + built from, at the tier the reader chose (FR-003); the reader sees their + summary verbatim. An unknown or expired handoff says so (FR-009), rather + than leaving an empty chat the reader will assume has the context. + """ + handoff = handoffs.get(handoff_id) + if handoff is None: + # Expired, never issued, or minted by another process -- the guest + # and logged-in chats do not share this in-memory store. + logger.info("handoff unavailable") + await cl.Message(content=seed.UNAVAILABLE).send() + return + + profile: str = (cl.user_session.get("chat_profile") or "").lower() + # `on_chat_start` sets `thread_id` from the session id. A claim arriving + # before it has run would otherwise seed a thread called "None". + thread_id: str = cl.user_session.get("thread_id") or cl.user_session.get("id") + try: + data = await seed.analysis_data(handoff) + seeded = await get_graph().seed_history( + profile, thread_id=thread_id, messages=seed.seeded_turn(handoff, data) + ) + except Exception: + # A failed fetch or graph update must not leave the reader in a chat + # that silently lacks the context they came for. + logger.exception("handoff seeding failed") + seeded = False + if not seeded: + logger.warning("handoff not seeded", extra={"profile": profile}) + await cl.Message(content=seed.UNAVAILABLE).send() + return + + logger.info( + "handoff claimed", + extra={"tier": handoff.tier, "with_data": data is not None}, + ) + await cl.Message(content=seed.shown_to_reader(handoff)).send() + + +@cl.on_window_message +async def on_window_message(message: object) -> None: + """Claim a "Continue in chat" handoff posted by this tab (spec 013). + + Chainlit forwards every message posted in the page, including this + server's own acknowledgement, so anything that is not a well-formed claim + is ignored. The tab retries until acknowledged, so one claim can arrive + several times; it is redeemed once per session and acknowledged every + time. + """ + handoff_id = claimed_id(message) + if handoff_id is None: + return + + claimed: set[str] = cl.user_session.get("handoff_claimed") or set() + if handoff_id not in claimed: + claimed.add(handoff_id) + cl.user_session.set("handoff_claimed", claimed) + await continue_from_handoff(handoff_id) + + await cl.send_window_message(acknowledgement(handoff_id)) + + @cl.on_message async def main(message: cl.Message) -> None: if await message_rate_limited(config): diff --git a/bin/chat-fastapi.py b/bin/chat-fastapi.py index 5b0d9743..729ab24b 100644 --- a/bin/chat-fastapi.py +++ b/bin/chat-fastapi.py @@ -16,6 +16,7 @@ from agent.registry import build_graph, set_graph from api.analysis_summary import router as analysis_summary_router from api.answer import router as answer_router +from api.handoff import router as handoff_router from util.caller_token import load_verifying_key from util.captcha_scope import is_captcha_exempt from util.embedding_environment import EmbeddingEnvironment @@ -72,6 +73,7 @@ async def lifespan(_app: FastAPI) -> AsyncIterator[None]: # is present -- a stricter bar than the answer endpoint's, because it discloses # the user's own uploaded analysis rather than public pathway text. app.include_router(analysis_summary_router, prefix=API_PREFIX) +app.include_router(handoff_router, prefix=API_PREFIX) CHAINLIT_URL = os.getenv("CHAINLIT_URL") CLOUDFLARE_SECRET_KEY = get_secret("CLOUDFLARE_SECRET_KEY") diff --git a/public/custom.js b/public/custom.js index df2a934d..3b0150d2 100644 --- a/public/custom.js +++ b/public/custom.js @@ -89,3 +89,41 @@ mo.observe(document.documentElement, { childList: true, subtree: true }); })(); + +/* + * Continue in chat (spec 013): claim a handoff carried in the URL fragment. + * + * The website opens /chat/guest/#handoff= in a new tab. The fragment + * never reaches a server, so the ID stays out of logs and Referer headers. + * Chainlit forwards every window message to the server over this tab's own + * socket, which binds the handoff to this tab -- a cookie would be shared by + * every tab and could seed the wrong one. + * + * Chainlit drops a message posted before its socket is up, so this retries + * until the server acknowledges, and gives up after a while rather than + * posting forever. + */ +(function () { + const match = /(?:^|&)handoff=([A-Za-z0-9_-]{22,128})(?:&|$)/.exec( + window.location.hash.slice(1) + ); + if (!match) return; + const id = match[1]; + + let acknowledged = false; + window.addEventListener('message', function (event) { + const data = event.data; + if (data && data.type === 'reactome-handoff-ack' && data.id === id) { + acknowledged = true; + } + }); + + let attempts = 0; + const timer = setInterval(function () { + if (acknowledged || ++attempts > 40) { + clearInterval(timer); + return; + } + window.postMessage({ type: 'reactome-handoff', id: id }, window.location.origin); + }, 500); +})(); diff --git a/specs/013-continue-chat-hand/spec.md b/specs/013-continue-chat-hand/spec.md new file mode 100644 index 00000000..b2265b8b --- /dev/null +++ b/specs/013-continue-chat-hand/spec.md @@ -0,0 +1,167 @@ +# Feature Specification: Continue in chat + +**Feature Branch**: `013-continue-chat-hand` + +**Created**: 2026-09-25 + +**Status**: Story 1 built and verified end to end; Stories 2 and 3 not started + +**Input**: Adam, 2026-09-25: "with the chat on both the search page and the analysis results we want there to be a button to go to the [chat] interface … two options … either the logged in version or the guest version of the chat app and have the context already be set with either the search results or the analysis summary with the summary data." And: "the chat should open in a new tab." + +## Context + +The website already shows a chatbot-written summary in two places, both +served by this repository: + +| where | endpoint | cached? | +|---|---|---| +| search page | `POST /api/answer` (spec 010) | no | +| analysis results | `POST /api/analysis/summary` (spec 011) | yes — `SummaryStore`, keyed `(token, release, tier)` | + +The feature is a **"Continue in chat"** button beside each summary. It opens +the chat app in a new tab, guest or logged in, with the conversation already +holding what the user was looking at, so their first message can be a +follow-up rather than a restatement. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 — Continue an analysis summary in the guest chat (Priority: P1) + +A user reads the summary of their enrichment analysis, clicks "Continue in +chat (guest)", and a new tab opens on the chat with that summary already in +the thread. They ask "which of these pathways involve TP53?" and get an +answer grounded in their analysis. + +**Why this priority**: analysis summaries are already cached and carry a +disclosure tier, so the handoff has everything it needs; and the guest chat +is the one path verifiable on beta. + +**Independent test**: from a real analysis token, the new tab's thread begins +with the same summary text the website showed, and a follow-up question is +answered using the analysis. + +### User Story 2 — Continue a search-page answer (Priority: P2) + +The same, from the search page's AI answer and its results. + +**Why P2**: `/api/answer` does not keep what it generated, so it needs a +store before the handoff can show the *same* answer (see FR-002). + +### User Story 3 — Continue in the logged-in chat (Priority: P2) + +The same handoff into `/chat/personal/`, so the conversation is kept in the +user's history. + +**Why P2, and why it cannot be verified on beta**: `/chat/personal` is not +deployed on beta — it needs a second container, Postgres, and OAuth whose +redirect URIs are bound to reactome.org. Verifiable in production only. + +## Requirements *(mandatory)* + +- **FR-001 Pass a reference, never the content.** No search result, summary + text or analysis data in a URL. URLs are written to nginx logs, leak to + other sites through `Referer`, and have length limits. The website asks the + chatbot for a short-lived handoff ID; the link carries only that ID. + +- **FR-002 The chat continues the summary the user saw, and does not + regenerate it.** Answers are not reproducible here — the same question on + the same build scores ~0.33 similarity run to run — so a regenerated summary + would greet the user with different text from the one they clicked from. + The handoff hands over the stored text. For analysis summaries it already + exists in `SummaryStore`; for search answers a store is required first. + +- **FR-003 The disclosure choice carries over.** A summary produced at the + `aggregate` tier continues at `aggregate`: the chat model receives the same + allow-listed view the summary was built from, and nothing wider. Continuing + in chat must not silently send OpenAI more than the user agreed to on the + website. The `identifiers` tier carries over only if it was chosen there. + +- **FR-004 The handoff ID travels in the URL fragment** + (`/chat/guest/#handoff=`). Browsers never send the fragment to a + server, so the ID does not reach nginx logs or `Referer` at all — FR-001 + enforced by the browser rather than by us remembering. + +- **FR-005 The handoff is bound to the tab, not the browser.** The link opens + in a new tab (FR-008), so a user can open several. A cookie is shared by + every tab: two handoffs in quick succession could overwrite each other + before the first tab connects, and that tab would open on the *wrong* + context. The ID is instead read by a script in the tab itself and sent over + that tab's own connection (FR-006). + +- **FR-006 Mechanism (to be prototyped).** Chainlit 2.11 does not give the app + the page URL on connect — its handler reads cookies only. It does offer + `custom_js` (a script injected into the chat page) and `@cl.on_window_message` + (a server hook for messages posted in the page). The script reads the + fragment and posts the ID; the hook redeems it and seeds the thread. The + risk to prove first: the post must arrive after the tab's socket is + connected, or it is lost. The script retries until the server acknowledges. + +- **FR-007 Valid for minutes, not once.** A single-use ID would give an empty + chat on a reload. The ID is redeemable for a short window (target: 15 + minutes) and then refused. It grants read access to one summary, so the + window is short and IDs are unguessable (≥128 random bits). + +- **FR-008 New tab.** The website opens the chat with `target="_blank"` and + `rel="noopener noreferrer"`, so the chat tab cannot reach back into the + website tab and receives no `Referer`. + +- **FR-009 An expired or unknown ID says so.** The chat opens normally and + tells the user the context could not be loaded, rather than silently + starting an empty conversation they will assume has the context. + +## Who builds what + +| | repo | +|---|---| +| the two buttons, the new-tab link, requesting a handoff ID | WebsiteAngular | +| `POST /api/handoff`, the store, the fragment script, the hook that seeds the thread | reactome_chatbot | + +## Out of scope + +- Seeding the chat with the raw search results or the full analysis result. + The chat receives what the summary was built from, bounded as it already + is, and can fetch more through its own tools. +- Carrying a handoff across devices or accounts. + +## Open questions + +- Whether the button should be two buttons (guest / logged in, as asked) or + one that the chat resolves after login. Two is what was asked for; recorded + so the choice is visible, not reopened. +- The handoff window length (FR-007) — 15 minutes is a guess to be revisited + once real use shows how long people take. + +--- + +## What was built, 2026-09-25 + +**Story 1 (analysis summary → guest chat) works end to end**, verified in a +headless browser against a real analysis on beta: the summary comes from the +real endpoint, the handoff is minted for it, the tab opens on *the same text* +the website was given, and a follow-up ("which pathway has the lowest FDR?") +is answered from the analysis -- naming its real top pathway with its exact +FDR. The control, the same question in a chat with no handoff, cannot answer +it at all. An identifiers-tier handoff is refused when the reader chose +aggregate, and an unknown ID says so. + +**The transport, measured.** A single `postMessage` in the first second after +load is lost 15 times out of 15 -- Chainlit drops window messages until its +socket is up -- and from two seconds on is claimed 6 of 6. `custom.js` +retries every 500 ms for up to 20 s. Two tabs opened together in one browser +each claimed only their own ID. + +**Story 3 needs a shared store before it can work.** The handoff store, like +the summary cache it copies from, is in process memory, and the guest and +logged-in chats are separate processes. A handoff minted by one cannot be +claimed by the other. The endpoint therefore returns only `path`, for the +chat served by the process that minted it; the first version also returned a +`personal_path` from the guest deployment, which could only ever have opened +on "couldn't load the summary". Two ways to do Story 3, to decide later: + +- a store both processes share (production already has Postgres for the + logged-in chat's LangGraph checkpoints), with the summary cache moved into + it too, so a handoff can cross processes and still continue the *same* + summary; or +- the website calls the logged-in deployment's own summary and handoff + endpoints -- simpler, but the summary would be generated a second time in + that process, and would not be the one the reader saw (FR-002). diff --git a/src/agent/graph.py b/src/agent/graph.py index 6d0aa94d..e7704300 100644 --- a/src/agent/graph.py +++ b/src/agent/graph.py @@ -9,6 +9,7 @@ from langchain_core.documents import Document from langchain_core.embeddings import Embeddings from langchain_core.language_models.chat_models import BaseChatModel +from langchain_core.messages import BaseMessage from langchain_core.runnables import RunnableConfig from langgraph.checkpoint.base import BaseCheckpointSaver from langgraph.checkpoint.memory import MemorySaver @@ -527,6 +528,29 @@ async def astream_answer( kind="done", state="answered" if answered else "nothing_found" ) + async def seed_history( + self, profile: str, *, thread_id: str, messages: list[BaseMessage] + ) -> bool: + """Put a turn into a thread's history as if it had been asked here. + + For spec 013's handoff: the reader's summary becomes the thread's + previous turn, so their first message can be a follow-up. Recorded as + having ended at `postprocess`, the graph's last node, so the next + `ainvoke` starts an ordinary new turn with this history in place. + + Returns False, and changes nothing, for an unknown profile. + """ + if self.graph is None: + self.graph = await self.initialize() + if profile not in self.graph: + return False + await self.graph[profile].aupdate_state( + RunnableConfig(configurable={"thread_id": thread_id}), + {"chat_history": messages}, + as_node="postprocess", + ) + return True + async def ainvoke( self, user_input: str, diff --git a/src/api/analysis_summary.py b/src/api/analysis_summary.py index e9d7eb5b..85528ffa 100644 --- a/src/api/analysis_summary.py +++ b/src/api/analysis_summary.py @@ -28,7 +28,7 @@ from agent.models import get_llm from analysis.client import current_release, fetch_not_found, fetch_result from analysis.disclosure import Tier, for_tier -from analysis.store import SummaryStore +from analysis.store import Stored, SummaryStore from analysis.summarise import ( INEXACT_COUNT_INSTRUCTION, NAMED_UNMATCHED_INSTRUCTION, @@ -61,6 +61,16 @@ #: a request, as the limiter does. _store = SummaryStore() + +def stored_summary(token: str, release: str, tier: str) -> Stored | None: + """A summary this endpoint generated and kept, if it still has it. + + For spec 013's handoff, which may only continue a summary the reader + has actually been shown. + """ + return _store.get(token, release, tier) + + SYSTEM_PROMPT = """ You explain a completed Reactome pathway-analysis result to the researcher who ran it. diff --git a/src/api/handoff.py b/src/api/handoff.py new file mode 100644 index 00000000..3c2c100b --- /dev/null +++ b/src/api/handoff.py @@ -0,0 +1,137 @@ +"""Mint a "Continue in chat" handoff for a summary the reader has seen. + +Spec 013. The website calls this when the reader clicks "Continue in chat" +beside an analysis summary, then opens `/chat/guest/#handoff=` (or the +personal path) in a new tab. The ID travels in the URL fragment, which +browsers never send to a server. + +**Only a summary that already exists can be handed off.** The summary is +looked up in the analysis-summary cache under the tier the reader chose. If +it is not there, this refuses rather than generating one. That enforces two +requirements with one check: + +- FR-002, continue what the reader *saw*: a summary that was never + generated was never seen. +- FR-003, never wider than they agreed to: a handoff at the `identifiers` + tier exists only if an `identifiers` summary does, which exists only if + the reader chose it on the website. + +Same authorisation bar as the summary endpoint -- caller token plus +evidence that a person is present -- because what it releases is that +reader's analysis summary. + +Plain JSON, not SSE: minting is a lookup, not a generation. +""" + +import os +import time +from typing import Any + +from fastapi import APIRouter, Request +from fastapi.responses import JSONResponse +from pydantic import BaseModel, Field + +from analysis.client import current_release +from analysis.disclosure import Tier +from api.analysis_summary import stored_summary +from handoff.store import DEFAULT_TTL_SECONDS, Handoff, handoffs +from util.caller_token import ( + TokenRejectedError, + human_presence_detail, + human_presence_reason, + verify, +) +from util.logging import logging +from util.rate_limit import identity_of, limiter_from_env + +logger = logging.getLogger(__name__) + +router = APIRouter() + +_limiter = limiter_from_env() + + +class HandoffRequest(BaseModel): + kind: str = Field(pattern="^analysis$") + token: str = Field(min_length=1, max_length=256) + #: The tier of the summary the reader was shown. Required, no default. + disclosure: Tier + caller_token: str = "" + + +def _refuse(status: int, reason: str, log: str) -> JSONResponse: + logger.info("handoff refused", extra={"reason": reason, "detail": log}) + return JSONResponse(status_code=status, content={"reason": reason}) + + +def own_chat_path() -> str: + """Where this process's chat is mounted, e.g. `/chat/guest`.""" + return (os.getenv("CHAINLIT_URI") or "/chat").rstrip("/") + + +@router.post("/handoff") +async def create_handoff(body: HandoffRequest, request: Request) -> JSONResponse: + verifying_key = getattr(request.app.state, "caller_token_key", None) + if not verifying_key: + return _refuse(403, "no_caller", "no verifying key on the app") + try: + claims = verify(body.caller_token, verifying_key) + except TokenRejectedError as rejected: + return _refuse(403, "no_caller", rejected.reason) + + presence = human_presence_reason(claims, time.time()) + if presence: + detail = ( + human_presence_detail(claims, time.time()) + if presence == "no_human" + else presence + ) + return _refuse(403, presence, detail) + + human_sub = claims.get("human_sub") + key = f"human:{human_sub}" if isinstance(human_sub, str) and human_sub else None + if not _limiter.allow(key or identity_of(claims, body.caller_token)): + return _refuse(429, "rate_limited", "rate limited") + + release = await current_release() + if not release: + # Summaries are cached per release, so without knowing the release + # there is no way to find the one the reader saw. Refusing is the + # honest answer; guessing a release could hand off a summary of a + # result the Analysis Service has since deleted. + return _refuse(503, "no_release", "current release unknown") + stored = stored_summary(body.token, release, body.disclosure) + if stored is None: + # Not generated, evicted, from a previous release, or requested at a + # tier the reader never chose. In every case there is nothing the + # reader has seen to continue, and generating one here would break + # both FR-002 and FR-003. + return _refuse( + 404, "no_summary", f"no stored summary at tier {body.disclosure}" + ) + + handoff_id = handoffs.put( + Handoff( + kind="analysis", + token=body.token, + release=release, + tier=body.disclosure, + summary=stored.text, + citations=stored.citations, + created_at=time.time(), + ) + ) + payload: dict[str, Any] = { + "id": handoff_id, + "expires_in": int(DEFAULT_TTL_SECONDS), + # The chat served by *this* process, and only that one. The store is + # in memory, and the guest and logged-in chats are separate + # processes, so a handoff minted here can only be claimed here. The + # first version also returned a `personal_path` from the guest + # deployment: a link that would always open on "couldn't load the + # summary", because the logged-in process had never heard of the ID. + # + # A fragment, not a query string: never sent to a server (FR-004). + "path": f"{own_chat_path()}/#handoff={handoff_id}", + } + return JSONResponse(status_code=200, content=payload) diff --git a/src/handoff/__init__.py b/src/handoff/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/handoff/seed.py b/src/handoff/seed.py new file mode 100644 index 00000000..ac8aae41 --- /dev/null +++ b/src/handoff/seed.py @@ -0,0 +1,95 @@ +"""What a claimed handoff puts into the conversation (spec 013). + +Two things, for two audiences, the same split `gsa` makes: + +- **The reader** sees the summary they clicked from, verbatim. +- **The model** gets that summary as its own previous turn, *plus the data it + was built from*, so a follow-up like "which of these involve TP53?" can be + answered from the analysis rather than from general knowledge. + +The seeded turn has the shape every real turn has -- `[HumanMessage, +AIMessage]` -- so nothing downstream has to know it was not typed. + +**The data is rebuilt at the handoff's tier, with the same functions the +summary endpoint uses** (`for_tier`, `prompt_input`, `fetch_not_found`), not a +copy of them. The chat model therefore receives exactly the allow-listed view +the summary was built from and nothing wider (FR-003). Rebuilding is safe +where regenerating the *text* would not be: an analysis result is a fixed +artefact, so the same token at the same tier yields the same data. +""" + +import json +from collections.abc import Awaitable, Callable +from typing import Any + +from langchain_core.messages import AIMessage, BaseMessage, HumanMessage + +from analysis.client import Fetched, fetch_not_found, fetch_result +from analysis.disclosure import for_tier +from analysis.summarise import prompt_input +from handoff.store import DEFAULT_TTL_SECONDS, Handoff + +#: What the reader asked for on the website, stated as what happened. +HUMAN_TURN = ( + "Summarise my Reactome pathway analysis. (Asked on the analysis results page.)" +) + +FetchResult = Callable[[str], Awaitable[Fetched]] +FetchUnmatched = Callable[[str], Awaitable[list[str] | None]] + + +async def analysis_data( + handoff: Handoff, + *, + fetch: FetchResult = fetch_result, + fetch_unmatched: FetchUnmatched = fetch_not_found, +) -> dict[str, Any] | None: + """The allow-listed data the summary was built from, or None if gone. + + None when the Analysis Service no longer has the result -- it deletes + results on a new release -- in which case the chat still continues the + summary, and says it cannot see the underlying numbers. + """ + fetched = await fetch(handoff.token) + if fetched.outcome != "ok" or fetched.result is None: + return None + data = prompt_input(for_tier(fetched.result, handoff.tier)) + if handoff.tier == "identifiers": + # Only at the tier the reader chose on the website, and only then. + unmatched = await fetch_unmatched(handoff.token) + if unmatched: + data["identifiers_not_found_names"] = unmatched + return data + + +def seeded_turn(handoff: Handoff, data: dict[str, Any] | None) -> list[BaseMessage]: + """The conversation turn the chat starts from.""" + if data is None: + appendix = ( + "\n\n(The analysis result behind this summary is no longer available " + "from Reactome, so only the summary above is known.)" + ) + else: + appendix = ( + "\n\nThe analysis data this summary was built from " + f"(disclosure tier: {handoff.tier}):\n" + f"```json\n{json.dumps(data, indent=1, default=str)}\n```" + ) + return [HumanMessage(HUMAN_TURN), AIMessage(handoff.summary + appendix)] + + +def shown_to_reader(handoff: Handoff) -> str: + """What the reader sees when the tab opens. Their own summary, verbatim.""" + return ( + "Continuing from your analysis summary:\n\n" + f"{handoff.summary}\n\n" + "---\nAsk a follow-up question about this analysis." + ) + + +UNAVAILABLE = ( + "I couldn't load the summary you came from — the link may have expired " + f"(they last {DEFAULT_TTL_SECONDS // 60:.0f} minutes). You can still ask me " + "anything here, or go back to the analysis page and choose *Continue in " + "chat* again." +) diff --git a/src/handoff/store.py b/src/handoff/store.py new file mode 100644 index 00000000..b4e5fb96 --- /dev/null +++ b/src/handoff/store.py @@ -0,0 +1,89 @@ +"""Handoffs waiting to be claimed by a chat tab (spec 013). + +A handoff is minted by the website for a summary the reader has just seen, +and claimed by the chat tab it opens. It carries a **copy of that summary's +text**, taken when it was minted. Two reasons: + +- The chat must continue the summary the reader saw, not regenerate one + (FR-002). Generation here is not reproducible -- the same question scores + ~0.33 similarity run to run -- so regenerating would greet the reader with + different text from the one they clicked from. +- The summary cache is bounded and process-local. Copying means the chat + still has the text if that cache has evicted it in the minutes between + mint and claim, and means the chat never reaches into another endpoint's + private state. + +**Redeemable repeatedly, for a short window** (FR-007). A single-use handoff +would give an empty chat when the tab is reloaded. Instead it can be claimed +any number of times until it expires. What it grants is read access to one +summary the reader already has, so the window is short and the ID cannot be +guessed. + +**In process, lost on deploy**, like the summary cache it copies from. The +API that mints and the Chainlit app that claims run in the same process +(`mount_chainlit` in `bin/chat-fastapi.py`), so module state is shared. +""" + +import secrets +import time +from collections import OrderedDict +from dataclasses import dataclass, field +from typing import Literal + +from analysis.disclosure import Tier + +#: Long enough to open the tab and reload it; short enough that a link which +#: escapes -- pasted into a message, left in history -- stops working soon. +DEFAULT_TTL_SECONDS = 15 * 60 + +#: Bounded because it lives for the life of the process. +DEFAULT_MAX_ENTRIES = 2048 + +Kind = Literal["analysis"] + + +@dataclass(frozen=True) +class Handoff: + kind: Kind + #: The analysis token the summary describes. + token: str + release: str + #: The disclosure tier the summary was *built* at. The chat continues at + #: exactly this tier (FR-003) -- never wider than the reader agreed to. + tier: Tier + summary: str + citations: tuple[tuple[str, str], ...] + created_at: float + + +def new_id() -> str: + """192 bits, URL-safe. Unguessable, and fits `handoff.window._ID`.""" + return secrets.token_urlsafe(24) + + +@dataclass +class HandoffStore: + ttl_seconds: float = DEFAULT_TTL_SECONDS + max_entries: int = DEFAULT_MAX_ENTRIES + _entries: OrderedDict[str, Handoff] = field(default_factory=OrderedDict) + + def put(self, handoff: Handoff) -> str: + handoff_id = new_id() + self._entries[handoff_id] = handoff + while len(self._entries) > self.max_entries: + self._entries.popitem(last=False) + return handoff_id + + def get(self, handoff_id: str, *, now: float | None = None) -> Handoff | None: + """The handoff, or None if unknown or expired. Expired ones are dropped.""" + found = self._entries.get(handoff_id) + if found is None: + return None + if (time.time() if now is None else now) - found.created_at > self.ttl_seconds: + del self._entries[handoff_id] + return None + return found + + +#: Shared by the API that mints and the chat hook that claims. +handoffs = HandoffStore() diff --git a/src/handoff/window.py b/src/handoff/window.py new file mode 100644 index 00000000..59ebfd01 --- /dev/null +++ b/src/handoff/window.py @@ -0,0 +1,57 @@ +"""The message a chat tab sends to claim its handoff, and the reply. + +Spec 013. The website opens the chat in a new tab at +`/chat/guest/#handoff=`. The ID is in the fragment because browsers never +send a fragment to a server: it cannot reach nginx logs or a `Referer`. + +Chainlit 2.11 does not give the app the page URL on connect -- its connect +handler reads cookies only -- and a cookie is shared by every tab, so two +handoffs opened in quick succession could seed the wrong one. Instead, a +script in the tab (`public/custom.js`) reads the fragment and posts the ID +with `window.postMessage`. Chainlit's page forwards `event.data` of *every* +message posted in the window to `@cl.on_window_message`, over that tab's own +socket. So the ID is bound to the tab that carried it. + +Two properties of that route shape this module: + +- **It forwards anything.** There is no origin or source check in Chainlit's + listener, and our own acknowledgement comes back through the same listener + (the server's reply is posted to `window.parent`, which for a top-level tab + is the tab itself). So every message is parsed strictly and anything that + is not a well-formed claim is ignored -- including our own ack. +- **It can drop messages.** If the socket is not connected yet, the page + discards the post. So the script retries until acknowledged, which means + the same claim can arrive more than once, and redeeming it must be + idempotent within a session. +""" + +import re +from typing import Any + +CLAIM_TYPE = "reactome-handoff" +ACK_TYPE = "reactome-handoff-ack" + +#: At least 128 bits of URL-safe randomness (22 base64url characters), and a +#: ceiling so a hostile page cannot post something enormous. +_ID = re.compile(r"^[A-Za-z0-9_-]{22,128}$") + + +def claimed_id(message: Any) -> str | None: + """The handoff ID in a window message, or None if it is not a claim. + + Returns None for everything that is not exactly a claim -- including the + acknowledgement this server sends, which Chainlit forwards straight back. + """ + if not isinstance(message, dict): + return None + if message.get("type") != CLAIM_TYPE: + return None + value = message.get("id") + if not isinstance(value, str) or not _ID.match(value): + return None + return value + + +def acknowledgement(handoff_id: str) -> dict[str, str]: + """What tells the tab's script to stop retrying.""" + return {"type": ACK_TYPE, "id": handoff_id} diff --git a/tests/api/test_handoff_endpoint.py b/tests/api/test_handoff_endpoint.py new file mode 100644 index 00000000..ef628175 --- /dev/null +++ b/tests/api/test_handoff_endpoint.py @@ -0,0 +1,219 @@ +"""`POST /api/handoff`: mint a Continue-in-chat handoff (spec 013). + +The endpoint's one real decision is what it refuses. It mints a handoff only +for a summary that already exists at the requested tier, which is how it +enforces both "continue what the reader saw" (FR-002) and "never wider than +they agreed to" (FR-003) with a single lookup. +""" + +import time + +import jwt +import pytest +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from analysis.store import SummaryStore +from api import handoff as endpoint +from handoff.store import HandoffStore +from util.caller_token import DEFAULT_AUDIENCE +from util.rate_limit import SlidingWindowLimiter + +PREFIX = "/chat/guest/api" +ANALYSIS = "MjAyNjA5MjExOTMyNTlfNjE%3D" + + +@pytest.fixture(scope="module") +def keys() -> tuple[str, str]: + private = Ed25519PrivateKey.generate() + return ( + private.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ).decode(), + private.public_key() + .public_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PublicFormat.SubjectPublicKeyInfo, + ) + .decode(), + ) + + +@pytest.fixture(autouse=True) +def stores(monkeypatch: pytest.MonkeyPatch) -> tuple[SummaryStore, HandoffStore]: + # Private stores per test: both are module state that outlives a request. + summaries = SummaryStore() + handoffs = HandoffStore() + monkeypatch.setattr("api.analysis_summary._store", summaries) + monkeypatch.setattr("api.handoff.handoffs", handoffs) + monkeypatch.setattr( + "api.handoff._limiter", SlidingWindowLimiter(limit=10_000, window=600.0) + ) + + async def _release() -> str: + return "97" + + monkeypatch.setattr("api.handoff.current_release", _release) + monkeypatch.setenv("CHAINLIT_URI", "/chat/guest") + return summaries, handoffs + + +def client(public_pem: str) -> TestClient: + app = FastAPI() + app.include_router(endpoint.router, prefix=PREFIX) + app.state.caller_token_key = public_pem + return TestClient(app) + + +def caller(private_pem: str, **claims: object) -> str: + payload: dict[str, object] = { + "iss": "reactome-website", + "aud": DEFAULT_AUDIENCE, + "exp": int(time.time()) + 300, + "sub": "visit-1", + "human": True, + "human_iat": int(time.time()) - 10, + } + payload.update(claims) + return jwt.encode(payload, private_pem, algorithm="EdDSA") + + +def mint(keys: tuple[str, str], disclosure: str = "aggregate", **claims: object): # type: ignore[no-untyped-def] + private, public = keys + return client(public).post( + f"{PREFIX}/handoff", + json={ + "kind": "analysis", + "token": ANALYSIS, + "disclosure": disclosure, + "caller_token": caller(private, **claims), + }, + ) + + +def test_mints_a_handoff_for_a_summary_the_reader_saw( + keys: tuple[str, str], stores: tuple[SummaryStore, HandoffStore] +) -> None: + summaries, handoffs = stores + summaries.put(ANALYSIS, "97", "aggregate", "Four pathways pass correction.", ()) + + response = mint(keys) + + assert response.status_code == 200 + body = response.json() + assert body["path"] == f"/chat/guest/#handoff={body['id']}" + stored = handoffs.get(body["id"]) + assert stored is not None + # A copy of the text the reader saw, not a reference to regenerate from. + assert stored.summary == "Four pathways pass correction." + assert stored.tier == "aggregate" + + +def test_the_id_travels_in_the_fragment_never_the_query( + keys: tuple[str, str], stores: tuple[SummaryStore, HandoffStore] +) -> None: + # A fragment is never sent to a server, so the ID cannot reach nginx + # logs or a Referer. A query string would reach both. + stores[0].put(ANALYSIS, "97", "aggregate", "Summary.", ()) + path = mint(keys).json()["path"] + assert "#handoff=" in path + assert "?" not in path + + +def test_the_path_is_this_processs_own_chat( + keys: tuple[str, str], + stores: tuple[SummaryStore, HandoffStore], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The store is in memory, and guest and logged-in chats are separate + processes, so a handoff can only be claimed by the chat that minted it. + The first version also returned a `personal_path` from the guest + deployment -- a link that could only ever open on "couldn't load".""" + stores[0].put(ANALYSIS, "97", "aggregate", "Summary.", ()) + monkeypatch.setenv("CHAINLIT_URI", "/chat/personal") + body = mint(keys).json() + assert body["path"].startswith("/chat/personal/#handoff=") + assert "personal_path" not in body + assert "guest_path" not in body + + +def test_refuses_when_no_summary_was_generated(keys: tuple[str, str]) -> None: + # Nothing the reader has seen, so nothing to continue -- and generating + # one here would break FR-002. + response = mint(keys) + assert response.status_code == 404 + assert response.json() == {"reason": "no_summary"} + + +def test_cannot_widen_the_disclosure_tier( + keys: tuple[str, str], stores: tuple[SummaryStore, HandoffStore] +) -> None: + """FR-003. The reader saw only the aggregate summary, so a request to + continue at the identifiers tier must fail: no such summary exists.""" + stores[0].put(ANALYSIS, "97", "aggregate", "Aggregate summary.", ()) + response = mint(keys, disclosure="identifiers") + assert response.status_code == 404 + assert stores[1].get("anything") is None + + +def test_a_summary_from_another_release_is_not_handed_off( + keys: tuple[str, str], stores: tuple[SummaryStore, HandoffStore] +) -> None: + # The Analysis Service deletes results on a new release; a summary of one + # describes an analysis that no longer exists. + stores[0].put(ANALYSIS, "96", "aggregate", "Old release.", ()) + assert mint(keys).status_code == 404 + + +def test_refuses_without_evidence_of_a_person( + keys: tuple[str, str], stores: tuple[SummaryStore, HandoffStore] +) -> None: + # Same bar as the summary endpoint: what this releases is a reader's + # analysis summary. + stores[0].put(ANALYSIS, "97", "aggregate", "Summary.", ()) + response = mint(keys, human=False) + assert response.status_code == 403 + + +def test_refuses_a_token_signed_by_someone_else( + keys: tuple[str, str], stores: tuple[SummaryStore, HandoffStore] +) -> None: + stores[0].put(ANALYSIS, "97", "aggregate", "Summary.", ()) + stranger = ( + Ed25519PrivateKey.generate() + .private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ) + .decode() + ) + response = client(keys[1]).post( + f"{PREFIX}/handoff", + json={ + "kind": "analysis", + "token": ANALYSIS, + "disclosure": "aggregate", + "caller_token": caller(stranger), + }, + ) + assert response.status_code == 403 + assert response.json() == {"reason": "no_caller"} + + +def test_refuses_an_unknown_kind(keys: tuple[str, str]) -> None: + private, public = keys + response = client(public).post( + f"{PREFIX}/handoff", + json={ + "kind": "search", + "token": ANALYSIS, + "disclosure": "aggregate", + "caller_token": caller(private), + }, + ) + assert response.status_code == 422 diff --git a/tests/handoff/test_handoff_seed.py b/tests/handoff/test_handoff_seed.py new file mode 100644 index 00000000..f5cf6d52 --- /dev/null +++ b/tests/handoff/test_handoff_seed.py @@ -0,0 +1,139 @@ +"""What a claimed handoff puts into the conversation, and what it must not.""" + +import asyncio +import time +from typing import Any + +from langchain_core.messages import AIMessage, HumanMessage + +from analysis.client import Fetched +from handoff import seed +from handoff.store import Handoff, HandoffStore + +RESULT: dict[str, Any] = { + "summary": {"token": "T", "type": "OVERREPRESENTATION"}, + "identifiersNotFound": 2, + "pathwaysFound": 3, + "pathways": [ + { + "stId": "R-HSA-1", + "name": "Cell Cycle", + "species": {"name": "Homo sapiens"}, + "entities": {"found": 5, "total": 10, "pValue": 1e-6, "fdr": 1e-4}, + } + ], +} + + +def handoff(tier: str = "aggregate", **overrides: Any) -> Handoff: + fields: dict[str, Any] = { + "kind": "analysis", + "token": "T", + "release": "97", + "tier": tier, + "summary": "Cell Cycle is the strongest signal.", + "citations": (), + "created_at": time.time(), + } + fields.update(overrides) + return Handoff(**fields) + + +class Spy: + def __init__(self, returns: Any) -> None: + self.calls = 0 + self.returns = returns + + async def __call__(self, _token: str) -> Any: + self.calls += 1 + return self.returns + + +def data_for(h: Handoff, fetch: Spy, unmatched: Spy) -> Any: + return asyncio.run(seed.analysis_data(h, fetch=fetch, fetch_unmatched=unmatched)) + + +class TestDisclosure: + def test_an_aggregate_handoff_never_fetches_the_readers_identifiers(self) -> None: + """FR-003, the one that matters. The reader chose aggregate on the + website; continuing in chat must not quietly go and get the + identifiers they declined to share.""" + unmatched = Spy(["MYSTERY1", "MYSTERY2"]) + data = data_for(handoff("aggregate"), Spy(Fetched("ok", RESULT)), unmatched) + + assert unmatched.calls == 0 + assert "identifiers_not_found_names" not in (data or {}) + + def test_an_identifiers_handoff_includes_them(self) -> None: + # The control: without it the test above would pass against code that + # never fetches identifiers at all. + unmatched = Spy(["MYSTERY1", "MYSTERY2"]) + data = data_for(handoff("identifiers"), Spy(Fetched("ok", RESULT)), unmatched) + + assert unmatched.calls == 1 + assert data["identifiers_not_found_names"] == ["MYSTERY1", "MYSTERY2"] + + +class TestTheSeededTurn: + def test_has_the_shape_of_a_real_turn(self) -> None: + turn = seed.seeded_turn(handoff(), {"pathways": []}) + assert [type(m) for m in turn] == [HumanMessage, AIMessage] + + def test_the_model_turn_carries_the_summary_the_reader_saw_verbatim(self) -> None: + turn = seed.seeded_turn(handoff(), {"pathways": []}) + assert str(turn[1].content).startswith("Cell Cycle is the strongest signal.") + + def test_the_model_turn_carries_the_data_it_was_built_from(self) -> None: + data = asyncio.run( + seed.analysis_data( + handoff(), fetch=Spy(Fetched("ok", RESULT)), fetch_unmatched=Spy(None) + ) + ) + text = str(seed.seeded_turn(handoff(), data)[1].content) + assert "Cell Cycle" in text + assert "aggregate" in text + + def test_a_result_that_is_gone_is_said_not_papered_over(self) -> None: + # The Analysis Service deletes results on a new release. + data = data_for(handoff(), Spy(Fetched("gone", None)), Spy(None)) + assert data is None + text = str(seed.seeded_turn(handoff(), None)[1].content) + assert "no longer available" in text + + +class TestTheReader: + def test_sees_their_summary_verbatim(self) -> None: + assert "Cell Cycle is the strongest signal." in seed.shown_to_reader(handoff()) + + def test_an_expired_link_says_so(self) -> None: + # FR-009: not a silent empty chat the reader assumes has the context. + assert "expired" in seed.UNAVAILABLE + + +class TestTheStore: + def test_a_handoff_can_be_claimed_repeatedly_within_its_window(self) -> None: + # FR-007: a reload re-claims, so single-use would empty the chat. + store = HandoffStore() + handoff_id = store.put(handoff()) + assert store.get(handoff_id) is not None + assert store.get(handoff_id) is not None + + def test_expires(self) -> None: + store = HandoffStore(ttl_seconds=60) + handoff_id = store.put(handoff(created_at=time.time() - 61)) + assert store.get(handoff_id) is None + + def test_an_unknown_id_is_none(self) -> None: + assert HandoffStore().get("never-issued") is None + + def test_ids_fit_what_the_tab_script_will_carry(self) -> None: + from handoff.window import claimed_id + + handoff_id = HandoffStore().put(handoff()) + assert claimed_id({"type": "reactome-handoff", "id": handoff_id}) == handoff_id + + def test_is_bounded(self) -> None: + store = HandoffStore(max_entries=3) + ids = [store.put(handoff()) for _ in range(5)] + assert store.get(ids[0]) is None + assert store.get(ids[-1]) is not None diff --git a/tests/handoff/test_handoff_window.py b/tests/handoff/test_handoff_window.py new file mode 100644 index 00000000..c7fa14ae --- /dev/null +++ b/tests/handoff/test_handoff_window.py @@ -0,0 +1,64 @@ +"""The claim a chat tab posts, and what the server accepts as one. + +Chainlit forwards every message posted in the page -- including this +server's own acknowledgement, which comes straight back through the same +listener -- so the parser's job is mostly to say no. + +The transport itself was verified in a headless browser against the real +Chainlit build: one claim per tab despite retries; two tabs opened together +in one browser each get only their own ID; a reload re-claims; and a single +post in the first second after load is lost 15 times out of 15, which is why +`public/custom.js` retries until acknowledged. +""" + +import secrets + +import pytest + +from handoff import window + +VALID = secrets.token_urlsafe(24) + + +def test_a_well_formed_claim_yields_its_id() -> None: + assert window.claimed_id({"type": window.CLAIM_TYPE, "id": VALID}) == VALID + + +def test_our_own_acknowledgement_is_not_a_claim() -> None: + # It comes back through Chainlit's listener. Treating it as a claim would + # re-seed the thread on every ack. + assert window.claimed_id(window.acknowledgement(VALID)) is None + + +@pytest.mark.parametrize( + "message", + [ + None, + "reactome-handoff", + ["reactome-handoff", VALID], + {"id": VALID}, + {"type": "something-else", "id": VALID}, + {"type": window.CLAIM_TYPE}, + {"type": window.CLAIM_TYPE, "id": 12345}, + {"type": window.CLAIM_TYPE, "id": "short"}, + {"type": window.CLAIM_TYPE, "id": "x" * 500}, + {"type": window.CLAIM_TYPE, "id": VALID + "/../../etc"}, + {"type": window.CLAIM_TYPE, "id": VALID + "