Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 102 additions & 1 deletion specs/011-summarise-analysis-results/quickstart.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ Expect `database/version` to match the release in the summary's `start` event.
|---|---|---|
| 1 | a token whose top pathways pass FDR | summary names those pathways, cites each by stable id |
| 2 | a list that hits nothing significant | summary says nothing passes correction; no p-value presented as a finding |
| 3 | mostly unmatched identifiers | summary reports the proportion and points at identifier type or species |
| 3 | mostly unmatched identifiers | summary reports the **count** and points at identifier type or species. Not the proportion: the aggregate result has no submitted-total in it, so a percentage would be invented (Phase 4) |
| 4 | `disclosure: aggregate` | the user's identifiers, filename, sample name and column labels appear in no outbound request |
| 5 | same token requested twice | byte-identical summary, `cached: true` on the second |
| 6 | an expired token | `state: not_found` |
Expand All @@ -75,3 +75,104 @@ Mechanical checks first, because they do not need judgement:
Then a curator reads a handful spanning strong, weak and empty results and says
whether each conveys how much to trust it. That is SC-006 and it cannot be
automated.

## Run against beta, 2026-09-21

Image `cc4315c`, release 97, inside the container -- the deployed code, the
installed bundle and the real Analysis Service. The caller-token check is
exercised separately over HTTP, because this service holds only the public
half of the keypair by design and cannot mint one to drive the full route.

| # | outcome |
|---|---|
| 1 | **pass** — summarised, 12 citations, every one present in the result. First token **1.8s and 2.0s on two runs of one token**: two samples, not a distribution, and not comparable to the answer endpoint's figures, which are medians over several questions. Quote it as "about two seconds on this token" or measure properly |
| 3 | **pass** — count reported, no invented percentage |
| 4 | **pass** — three aggregate payloads checked, including the expression one carrying `Patient_001_tumour`; no forbidden field and no user label in any of them. The disclosing payload *does* carry the names, so the check is not passing because nothing was sent |
| 5 | **pass** — second request `cached: true` and byte-identical |
| 6 | **pass** — unknown token gives `not_found` |
| 7 | **pass** — a constructed pre-release token gives `gone` on the deployed build |
| 9 | **pass** — verified over HTTP on the deployed route: no caller token gives `refused` / `no_caller`, HTTP 200 |

Also checked, beyond the table: an `EXPRESSION` result referred to `column 1`,
`column 2`, `column 3` and no other form, and the `identifiers` tier named the
reader's unmatched identifiers.

**Three scenarios were not run, and the reason is that they cannot be
constructed here rather than that they were skipped.**

- **2, nothing significant** — needs an identifier list that hits no pathway
past correction. Every list tried produced significant hits, and inventing
one by editing a result would test the code against a fixture rather than
the service. Covered by unit tests on the verdict instead.
- ~~**7, `gone`**~~ — **this turned out to be constructible, and now passes.**
See below.
- **8, ReactomeGSA** — needs a GSA analysis, which is a different service.
Detection is unit-tested against all three GSA type values.

Scenario 5 across a restart is expected to fail until a durable store exists
(research D4), and was not attempted for that reason.

**The first run of scenario 4 proved nothing**, and the correction is worth
keeping because the mistake is easy to repeat. The disclosure check ran
before the expression summary, so the payload carrying the column labels --
the strongest disclosure risk in the feature -- was not among those examined.
It passed by ordering rather than by evidence. The check now runs last, over
every aggregate payload produced, and asserts in the other direction too:
the disclosing tier's payload must contain the names, or a clean aggregate
check might only mean nothing was ever sent.

### Scenario 7 is constructible after all

An analysis token is base64 of `YYYYMMDDHHMMSS_counter` --
`MjAyNjA5MTkxODExNDJfMTE=` decodes to `20260919181142_11`. So a token for any
past moment can be made without the service having issued it.

Measured on 2026-09-21, the service distinguishes three ages rather than two:

| token timestamp | response |
|---|---|
| 2026-09-13 00:00:01 and earlier | 404 |
| **2026-09-13 12:00:00 onward** | **410** |

**It is a rolling cutoff measured in time, not a date boundary.** The two
readings above are the same calendar day: one second past midnight is 404 and
noon is 410, so the window is roughly the last eight days and it slides
forward continuously. The website session independently confirmed the same
behaviour on production, and found the boundary a day earlier than a first
sample here had suggested — which is the argument for deriving it rather than
quoting it.

So there is a retention window in which a result deleted by a release is
still *remembered as deleted*, and anything older is simply unknown. A token
inside that window produces a real 410, and the deployed pipeline maps it to
`gone`:

```
python -c "import base64; print(base64.b64encode(b'20260915120000_1').decode())"
# -> MjAyNjA5MTUxMjAwMDBfMQ== -> outcome: gone
```

This matters beyond ticking the scenario off. The `gone` path would otherwise
have been first exercised for real during a release, which is the worst
moment to discover a handling bug in it.

**Bisect for the boundary at run time; never hardcode a token.** The cutoff
slides forward daily, so any fixed timestamp eventually falls out of the
window and starts returning 404 -- and a test that does that does not fail,
it quietly starts asserting `not_found` while still passing. That is the same
shape as everything else this feature has had to guard against: a check that
goes on passing after it has stopped testing the thing.

**Two people measured this and agreed, and both were wrong.** One read the
boundary as 09-13 and the other as 09-14; the answer is that there is no
boundary day at all. Neither had tested *within* a day, because both were
picturing a date cutoff and so both asked "which day" rather than "which
shape". The agreement felt like corroboration and was not — it was two
measurements taken the same way, and a shared assumption survives any number
of those. It took testing at a finer grain than the question assumed.

The assumption worth naming, because it cost more than the boundary did: this
scenario was recorded as untestable on the belief that only the service can
mint a token. That was never decided or written down anywhere, and it was
load-bearing for a plan in which `gone` would first be exercised by a curator
during a release. Nothing about it looked like a question.
4 changes: 2 additions & 2 deletions specs/011-summarise-analysis-results/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ carried. Everything in this phase can be built and tested before that lands.

- [x] T034 [P] Bound the summary in `src/api/analysis_summary.py` as the answer endpoint is, so a stuck upstream cannot hold a connection. **The bound existed since Phase 3 and was untested until now**; the test also pins that a truncated summary is never stored, which would otherwise be served forever
- [x] T035 [P] Log an abandoned summary stream in `src/api/analysis_summary.py`, as the answer endpoint does, so a caller that starts summaries it does not want is visible
- [ ] T036 Run the [quickstart](./quickstart.md) scenarios against beta with a real analysis token and record the outcome, including first-token timing
- [ ] T037 Tell the website session the endpoint exists, what it does not yet do, and the `gone` outcome they must handle — only once it is live on beta, not when it merges
- [x] T036 Run the [quickstart](./quickstart.md) scenarios against beta with a real analysis token and record the outcome, including first-token timing. **Seven of nine pass on image `cc4315c`; two cannot be constructed** (a null result, a ReactomeGSA analysis — the `gone` token turned out to be constructible from the timestamp the token encodes) and the quickstart says so rather than marking them skipped. First token 1.8s and 2.0s on two runs of a single token — two samples, not a distribution
- [x] T037 Told the website session on 2026-09-21, once it was live on beta (image `cc4315c`) rather than when it merged. Sent: the three terminal states they must handle (`gone` distinct from `not_found`, `unsupported` for the three ReactomeGSA types, `refused` with its `reason`), the three scenarios that could not be verified live and are unit-tested only, and that summaries are lost on deploy by design

## Dependencies

Expand Down
Loading