Spec 011: summarise analysis results

specs/011-summarise-analysis-results/checklists/requirements.md

@@ -0,0 +1,51 @@
+# Specification Quality Checklist: Summarise analysis results
+
+**Purpose**: Validate specification completeness and quality before proceeding to planning
+**Created**: 2026-09-18
+**Feature**: [spec.md](../spec.md)
+
+## Content Quality
+
+- [x] No implementation details (languages, frameworks, APIs)
+- [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] Success criteria are technology-agnostic (no implementation details)
+- [x] All acceptance scenarios are defined
+- [x] Edge cases are identified
+- [x] Scope is clearly bounded
+- [x] Dependencies and assumptions identified
+
+## Feature Readiness
+
+- [x] All functional requirements have clear acceptance criteria
+- [x] User scenarios cover primary flows
+- [x] Feature meets measurable outcomes defined in Success Criteria
+- [x] No implementation details leak into specification
+
+## Notes
+
+Both clarifications were answered by Adam on 2026-09-18 and are now requirements,
+not assumptions:
+
+- **Privacy** — summarising is **opt-in** (FR-011), the user **chooses what is
+  shared** with at least one useful option that discloses no identifiers (FR-012),
+  and a **person must be shown to be present** (FR-013). That last is a stricter
+  bar than the answer endpoint's caller token, which asserts service identity and
+  deliberately says nothing about humanity — so this feature cannot ride the
+  ungated search path.
+- **Stability** — "do our best and be transparent" became FR-014 (the same token
+  yields the same summary, by reuse) and FR-015 (say it is generated, and that
+  regenerating may differ). Stability comes from storing the summary against a
+  fixed artefact, not from pretending the generator is deterministic.
+
+ReactomeGSA remains deferred in Assumptions: separate service, separate result
+shape.
+
+All checklist items pass. Ready for planning.

specs/011-summarise-analysis-results/contracts/summary_endpoint.md

@@ -0,0 +1,85 @@
+# Contract: the analysis summary endpoint
+
+Draft. Not implemented, and not yet agreed with the website repo. The parts
+marked **open** need their agreement before anything is built against them.
+
+## Request
+
+```
+POST {CHAINLIT_URI}/api/analysis-summary
+Content-Type: application/json
+
+{ "token": "MjAyNjA5MTgxMjM0NTY",
+  "caller_token": "<the website's EdDSA JWT, as for /api/answer>",
+  "disclosure": "aggregate" }
+```
+
+`disclosure` is `aggregate` or `identifiers`, and is **required** — there is no
+default, because a default is not a choice. `aggregate` never transmits the user's
+identifiers, filenames, sample names or expression column labels.
+
+**Open**: how the caller demonstrates a person is present. Spec 010's D1 settled
+that `caller_token` asserts service identity and says nothing about humanity, so
+it cannot carry this on its own. The likely shape is an additional claim minted
+after the Turnstile check the website already performs on the chat, but that is
+theirs to agree.
+
+## Response: Server-Sent Events
+
+The same framing as `/api/answer`, for the same reasons — a summary takes
+comparable time, and a caller must be able to ignore it safely.
+
+```
+event: start
+data: {"release": 97, "analysis_type": "OVERREPRESENTATION", "cached": false}
+
+event: token
+data: {"text": "Of the 312 pathways hit, four remain significant after "}
+
+event: citation
+data: {"st_id": "R-HSA-109581", "display_name": "Apoptosis"}
+
+event: done
+data: {"state": "summarised", "seconds": 6.2}
+```
+
+`state` is one of `summarised`, `not_found`, `gone`, `unsupported`, `refused`,
+`failed`. Anything but `summarised` means render no summary. Always HTTP 200 —
+never an error code, so the analysis page cannot be broken by this service.
+
+`cached` on `start` says whether this text was generated now or reused. It exists
+because the interface must not imply determinism it does not have: a reader who
+regenerates may get different wording, and `cached: false` is when that happens.
+
+## What a caller must handle
+
+**`gone` is not `not_found`.** The Analysis Service deletes results on a new
+release and says so with 410. A reader whose token returns `gone` should be told
+their analysis predates the current release and to run it again — that is an
+action, where `not_found` is a dead end.
+
+**Summaries are stable per `(token, release, disclosure)`**, by storage rather
+than by the generator being deterministic. The same request returns the same text
+until the release changes. After a release the stored summary is discarded,
+because the result it described has been too.
+
+**An aggregate summary and a disclosing one are different artefacts.** They are
+stored separately and must not be presented as the same summary at different
+detail.
+
+## What this endpoint will not do
+
+- Run, re-run or re-implement an analysis. It summarises a result that exists.
+- State a statistic the result does not contain.
+- Describe the lowest p-value as a finding when nothing passes correction.
+- Summarise a ReactomeGSA result. It recognises one and returns `unsupported`.
+- Read from production. Results come from beta's Analysis Service.
+
+## Open questions for the website side
+
+1. How human presence is asserted (above) — the blocker.
+2. Where the disclosure choice is presented, and how the two options are described
+   so a user can tell what they are trading.
+3. Whether the analysis page shows a summary automatically once consent exists, or
+   requires the ask each time. The spec requires opt-in per request; if that is
+   wrong for the page, it is a spec change rather than an implementation one.

specs/011-summarise-analysis-results/data-model.md

@@ -0,0 +1,88 @@
+# Data model: summarising analysis results
+
+Everything here is read from the Analysis Service or produced by this feature.
+Nothing about an analysis is stored except the summary we generate.
+
+## Read from the Analysis Service
+
+### AnalysisResult (retrieved by token)
+
+| field | used for | tier |
+|---|---|---|
+| `summary.type` | which reading applies: `OVERREPRESENTATION`, `EXPRESSION`, `SPECIES_COMPARISON` | aggregate |
+| `summary.gsaMethod`, `summary.gsaToken` | detect ReactomeGSA and decline (D8) | aggregate |
+| `summary.species`, `speciesName` | which organism was analysed | aggregate |
+| `summary.projection`, `interactors`, `includeDisease` | caveats the summary must respect | aggregate |
+| `summary.fileName`, `sampleName` | **never sent** — user-supplied free text | identifier |
+| `pathways[].stId` | the citation; how a reader opens the pathway | aggregate |
+| `pathways[].name` | what the summary calls it | aggregate |
+| `pathways[].entities.found` / `.total` / `.ratio` | how much of the pathway was hit | aggregate |
+| `pathways[].entities.pValue` / `.fdr` | significance, before and after correction | aggregate |
+| `pathways[].entities.curatedFound` / `.interactorsFound` | whether a hit rests on curated data or inferred interactors | aggregate |
+| `pathways[].entities.exp[]` | expression values per column (story 4) | aggregate |
+| `expression.columnNames` | **never sent** — user-supplied column labels | identifier |
+| `expression.min` / `.max` | the range values sit in | aggregate |
+| `resourceSummary` | which identifier resources matched, the usual clue to a mismatch | aggregate |
+| `speciesSummary` | species breakdown | aggregate |
+| `identifiersNotFound` | how many did not match (a count, not the identifiers) | aggregate |
+| `pathwaysFound` | how many pathways were hit at all | aggregate |
+| `warnings` | what the service itself flagged | aggregate |
+
+### Not-found identifiers (retrieved only on explicit request)
+
+`GET /token/{token}/notFound` — the user's own unmatched identifiers. Identifier
+tier. Fetched only when the user has chosen the disclosing option, and never
+otherwise.
+
+### Release
+
+`GET /database/version` — the release the Analysis Service is currently serving.
+Part of the stored summary's key, because results are deleted on a release change.
+
+## Produced by this feature
+
+### DisclosureChoice
+
+What the user agreed to share, chosen per request.
+
+| field | values | meaning |
+|---|---|---|
+| `tier` | `aggregate` \| `identifiers` | which fields may be sent |
+| `asked_at` | timestamp | when the user chose; absent means no consent and no summary |
+
+`aggregate` is the default and the only one that can be pre-selected. `identifiers`
+is never a default.
+
+### AnalysisSummary (ours, not the service's)
+
+| field | meaning |
+|---|---|
+| `token` | the analysis summarised |
+| `release` | the release it was generated against; with `token`, the storage key |
+| `tier` | which disclosure tier produced it — an aggregate summary and a disclosing one are different artefacts and must not be interchanged |
+| `text` | the summary itself |
+| `citations` | the pathways discussed, by stable id |
+| `generated_at` | when, so the interface can say how old it is |
+
+**Key**: `(token, release, tier)`. A release change invalidates every summary for
+the prior release, matching the service deleting the results themselves.
+
+### Citation
+
+Reuses the answer endpoint's shape exactly: `st_id` with a display name, resolving
+to `reactome.org/content/detail/<st_id>`. A summary cites only pathways present in
+the result it describes — an invented or mismatched id is the failure this pins.
+
+## Outcomes
+
+| outcome | when |
+|---|---|
+| `summarised` | a summary was produced |
+| `not_found` | the token matches no result (404) |
+| `gone` | the result was deleted by a release (410) — tell the user to re-run |
+| `unsupported` | a ReactomeGSA result, recognised and declined |
+| `refused` | no verified caller, or no evidence of a person |
+| `failed` | anything else |
+
+`gone` is deliberately distinct from `not_found`: one is a dead end, the other has
+an action attached.

specs/011-summarise-analysis-results/plan.md

@@ -0,0 +1,115 @@
+# Implementation Plan: Summarise analysis results
+
+**Branch**: `011-summarise-analysis-results` | **Date**: 2026-09-18 | **Spec**: [spec.md](./spec.md)
+**Input**: Feature specification from `specs/011-summarise-analysis-results/spec.md`
+
+## Summary
+
+Take an analysis token, fetch the completed result from beta's Analysis Service,
+and produce a short readable account of what it says — which pathways came out on
+top, whether they survive multiple-testing correction, why identifiers went
+unmatched, and what the numbers mean. Cite every pathway by stable id. Store the
+summary against `(token, release, tier)` so the same token yields the same text,
+and say plainly that it was generated.
+
+The feature never runs an analysis. It never sends the user's identifiers unless
+they choose that, and never their filename, sample name or column labels at all
+under the default choice.
+
+## Technical Context
+
+**Language/Version**: Python 3.12, as the rest of the service
+**Primary Dependencies**: the existing LangGraph answer surface, caller-token
+verification, `AnchorStripper`, and the SSE response shape — all reused, none
+rebuilt
+**Storage**: summaries keyed `(token, release, tier)`. **No durable store exists on
+beta today** — `POSTGRES_LANGGRAPH_DB` is unset there and LangGraph already falls
+back to `MemorySaver`. First increment may hold summaries in process memory;
+persistence is follow-up work (research D4)
+**Testing**: pytest, with the suite's existing rule that it passes with no API keys
+set. The disclosure guarantee is tested by recording outbound requests, not by
+inspecting output
+**Target Platform**: the same container that serves `/chat` and `/api/answer`
+**Project Type**: single service
+**Performance Goals**: comparable to the answer endpoint — first token in about
+ten seconds. A stored summary returns immediately, which is most requests after
+the first
+**Constraints**: read from beta, never production; browser-like `User-Agent` on
+outbound calls or the site's automation blocking returns 403 HTML; never state a
+statistic the result does not contain
+**Scale/Scope**: four analysis types, of which three are summarised and one
+(ReactomeGSA) is recognised and declined
+
+## Constitution Check
+
+| Principle | Status | Note |
+|---|---|---|
+| I — verify the path a user takes | **Pass, with a named risk** | The endpoint must be exercised over HTTP on the real mounted app, as spec 010's was. The captcha middleware and the new human-presence bar interact only on the served path, and that is exactly where spec 010's route check found a problem that isolated tests could not. |
+| II — measure retrieval changes, do not argue | **Not applicable** | This feature retrieves nothing from the vector store. It reads a completed analysis result. |
+| III — characterization tests pin behaviour | **Pass** | The behaviours worth pinning are negative: what is *not* sent under the aggregate tier, that `gone` is distinct from `not_found`, and that nothing is said when nothing passes correction. |
+| IV — fail loudly, never quietly differently | **Pass, and it cuts both ways** | Misconfiguration must stop the process, as the caller-token key already does. But a runtime failure must be quiet to the caller — a terminal state, never an HTTP error — because an analysis page must not break because this service did. |
+| V — derive from the source of truth | **Pass, and it drives a decision** | The release is read from `GET /database/version`, not hardcoded or copied from the embeddings bundle. That matters because the Analysis Service deletes results on a release change, so the release is also the cache-invalidation key. |
+| VI — bias to doing over filing | **Pass** | The blocker (how human presence is asserted) is recorded as a task with a named counterpart, not filed as a question and left. |
+| VII — parked is not dead | **Pass** | ReactomeGSA is explicitly deferred with the reason, not silently omitted. |
+
+**No violations requiring justification.**
+
+## Project Structure
+
+### Documentation (this feature)
+
+```
+specs/011-summarise-analysis-results/
+├── spec.md
+├── plan.md              # this file
+├── research.md          # D1-D8, measured against beta's API
+├── data-model.md        # what is read, what is produced, and the disclosure tiers
+├── quickstart.md        # how to validate it with a real analysis token
+├── checklists/
+│   └── requirements.md
+└── contracts/
+    └── summary_endpoint.md
+```
+
+### Source Code (repository root)
+
+```
+src/
+├── api/
+│   ├── answer.py             # existing; patterns reused, not modified
+│   └── analysis_summary.py   # new: the endpoint
+├── analysis/                 # new
+│   ├── client.py             # fetch a result by token from beta; 404 vs 410
+│   ├── disclosure.py         # the field allow-list that defines the aggregate tier
+│   ├── summarise.py          # build the prompt input from a result
+│   └── store.py              # summaries keyed (token, release, tier)
+└── util/
+    └── caller_token.py       # existing; extended if human presence rides the token
+
+tests/
+├── api/
+│   └── test_analysis_summary.py
+└── analysis/
+    ├── test_disclosure.py    # what must never be sent
+    ├── test_client.py        # 404, 410, and the release
+    └── test_store.py         # stability, and invalidation on release change
+```
+
+`src/analysis/` is separate from `src/agent/` because none of this touches the
+graph or retrieval. It reads an external result and shapes it for a prompt.
+
+## Complexity Tracking
+
+One thing here is more complex than it first appears, and it is worth naming
+rather than discovering.
+
+**The disclosure tier is a field allow-list, not a field denial.** The obvious
+implementation — "strip the identifiers" — misses `summary.fileName`,
+`summary.sampleName` and `expression.columnNames`, all of which are user-supplied
+free text that can carry a lab's unpublished filename or a patient sample label. A
+denial list is wrong by default whenever the Analysis Service adds a field; an
+allow-list is only ever wrong by omission, which costs a missing sentence rather
+than a disclosure.
+
+Everything else is deliberately unambitious: one external call, one prompt, one
+store keyed by three values.