From 3b2c79254230b9cced79afb67a6dda1528970eb9 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 25 Sep 2026 19:59:44 +0000 Subject: [PATCH 1/4] Run an over-representation analysis on a gene list typed into the chat Asked "can we do a gsa analysis in the chat. I want to do it with genes TP53, ERBB2 and RUNX2", the chat replied with upload instructions for an expression matrix. A gene list is not a matrix; what Reactome runs on one is over-representation, and the Analysis Service runs it in a second. The chat now recognises a request to analyse two or more identifiers, submits them to the Analysis Service, and replies with the matched count, the top pathways by FDR, and a Pathway Browser link on the service that holds the token. The exchange is seeded as the thread's previous turn so follow-ups are answered from the result. Co-Authored-By: Claude Opus 5.5 --- bin/chat-chainlit.py | 59 ++++++++ src/analysis/client.py | 79 +++++++++++ src/analysis/gene_list.py | 210 ++++++++++++++++++++++++++++ src/gsa/chat.py | 23 +--- src/util/markdown.py | 13 ++ tests/analysis/test_gene_list.py | 226 +++++++++++++++++++++++++++++++ tests/gsa/test_gsa_chat.py | 14 +- 7 files changed, 601 insertions(+), 23 deletions(-) create mode 100644 src/analysis/gene_list.py create mode 100644 src/util/markdown.py create mode 100644 tests/analysis/test_gene_list.py diff --git a/bin/chat-chainlit.py b/bin/chat-chainlit.py index 2d1ecfd..6dd265c 100644 --- a/bin/chat-chainlit.py +++ b/bin/chat-chainlit.py @@ -12,11 +12,19 @@ from chainlit.types import ThreadDict from dotenv import load_dotenv from langchain_community.callbacks import OpenAICallbackHandler +from langchain_core.messages import AIMessage, HumanMessage from agent.profile_names import ProfileName from agent.profiles import get_chat_profiles from agent.profiles.base import OutputState from agent.registry import get_graph +from analysis import gene_list +from analysis.client import ( + MAX_SUBMITTED_IDENTIFIERS, + fetch_not_found, + pathway_browser_url, + submit_identifiers, +) from gsa.chainlit_flow import ( Attachment, matrix_attachment, @@ -258,6 +266,49 @@ async def continue_from_handoff(handoff_id: str) -> None: await cl.Message(content=seed.shown_to_reader(handoff)).send() +async def run_gene_list_analysis(text: str, identifiers: list[str]) -> None: + """Over-representation on a pasted gene list, and seed it for follow-ups.""" + submitted = identifiers[:MAX_SUBMITTED_IDENTIFIERS] + result = await submit_identifiers(submitted) + if result is None: + await cl.Message(content=gene_list.FAILED).send() + return + not_found = result.result.get("identifiersNotFound") + unmatched = ( + await fetch_not_found(result.token) + if isinstance(not_found, int) and not_found > 0 + else None + ) + reply = gene_list.describe_overrepresentation( + submitted, + result.result, + pathway_browser_url(result.token), + unmatched, + truncated=len(identifiers) > len(submitted), + ) + await cl.Message(content=reply.text).send() + logger.info( + "gene list analysed", + extra={"submitted": len(submitted), "has_pathways": reply.has_pathways}, + ) + if not reply.has_pathways: + return + # The exchange becomes the thread's previous turn, so "which of these + # involve TP53?" is answered from this result. The reader typed the list + # into this chat, whose every message goes to the model anyway. + profile: str = (cl.user_session.get("chat_profile") or "").lower() + thread_id: str = cl.user_session.get("thread_id") or cl.user_session.get("id") + try: + await get_graph().seed_history( + profile, + thread_id=thread_id, + messages=[HumanMessage(content=text), AIMessage(content=reply.text)], + ) + except Exception: + # The reader has their result; only follow-ups lose it. + logger.exception("gene list seeding failed") + + @cl.on_window_message async def on_window_message(message: object) -> None: """Claim a "Continue in chat" handoff posted by this tab (spec 013). @@ -303,6 +354,14 @@ async def main(message: cl.Message) -> None: # grounded in the user guide, which describes the website's GSA page, so # it said this chat could not do it. Answered here instead -- chat only, # because the same answer path serves the search page, which cannot. + # A gene list is not a matrix: run what Reactome runs on a list, + # over-representation, rather than explaining how to upload a matrix. + # Before the GSA check, because the request that prompted this said "gsa". + identifiers = gene_list.gene_list_request(message.content or "") + if identifiers is not None: + await run_gene_list_analysis(message.content, identifiers) + return + if asks_to_run_gsa(message.content or ""): await cl.Message(content=HOW_TO_RUN_GSA).send() return diff --git a/src/analysis/client.py b/src/analysis/client.py index 74f2ae8..0e12b9a 100644 --- a/src/analysis/client.py +++ b/src/analysis/client.py @@ -235,3 +235,82 @@ async def fetch_not_found( for entry in payload if isinstance(entry, dict) and entry.get("id") ][:limit] + + +#: A gene list pasted into a chat is short; this bounds a hostile paste. +MAX_SUBMITTED_IDENTIFIERS = 3000 + + +@dataclass(frozen=True) +class Submitted: + """A new over-representation analysis, and the first page of its result.""" + + token: str + result: dict[str, Any] + + +async def submit_identifiers( + identifiers: list[str], + *, + page_size: int = 10, + client: httpx.AsyncClient | None = None, +) -> Submitted | None: + """Run an over-representation analysis on a list of identifiers. + + Measured against beta on 2026-09-25 rather than read off the docs: the + reply is JSON (unlike ReactomeGSA's `text/plain`), the token is in + `summary.token` and arrives already URL-encoded (`...MTU%3D`), and + `pathways` is ranked by the `sortBy` we ask for. Projected to human. + + None when the service refuses or answers in a shape without a usable + token -- the caller says so rather than inventing a result. + """ + ids = [i for i in identifiers if i][:MAX_SUBMITTED_IDENTIFIERS] + if not ids: + return None + owned = client is None + client = client or httpx.AsyncClient(timeout=TIMEOUT_SECONDS) + try: + response = await client.post( + f"{base_url()}/identifiers/projection", + content="\n".join(ids), + headers={**_headers(), "Content-Type": "text/plain"}, + params={ + "pageSize": page_size, + "page": 1, + "sortBy": "ENTITIES_FDR", + "order": "ASC", + }, + ) + if response.status_code != 200: + logger.warning( + "identifier submission refused", + extra={"status": response.status_code}, + ) + return None + result = response.json() + except Exception as exc: + # A timeout or a non-JSON body is "no result", said to the reader -- + # not an exception out of the chat handler. + logger.warning("identifier submission failed: %s", type(exc).__name__) + return None + finally: + if owned: + await client.aclose() + summary = result.get("summary") if isinstance(result, dict) else None + token = summary.get("token") if isinstance(summary, dict) else None + if not isinstance(token, str) or not is_well_formed(token): + logger.warning("identifier submission returned no usable token") + return None + return Submitted(token=token, result=result) + + +def pathway_browser_url(token: str) -> str: + """The result in the Pathway Browser *of the service that holds it*. + + The token only exists in the Analysis Service that issued it -- beta, by + default -- so a reactome.org link would open on a token production has + never seen. + """ + site = base_url().removesuffix("/AnalysisService") + return f"{site}/PathwayBrowser/#/DTAB=AN&ANALYSIS={token}" diff --git a/src/analysis/gene_list.py b/src/analysis/gene_list.py new file mode 100644 index 0000000..312ec64 --- /dev/null +++ b/src/analysis/gene_list.py @@ -0,0 +1,210 @@ +"""A gene list typed into the chat, run as an over-representation analysis. + +Asked in the chat, verbatim: "can we do a gsa analysis in the chat. I want +to do it with genes TP53, ERBB2 and RUNX2". It got upload instructions for an +expression matrix -- true, and no use: three genes are not a matrix. What +Reactome runs on a list of genes is over-representation, and the Analysis +Service will run it in a second, so the chat now does. + +Pure functions, like `gsa.chat`: what counts as a gene list, and what the +reader is told, are tested here; the handler is wiring. + +**Recognising a request is a heuristic, so it is narrow on purpose.** It +needs all three of: an analysis term, a request to do something, and at +least `MIN_IDENTIFIERS` identifier-shaped tokens. A question *about* genes +("can you explain how TP53 and MDM2 interact?") has no analysis term and +still goes to the model. A single gene is not a list: an enrichment of one +gene is every pathway that gene is in, which the model answers better. +""" + +import re +from dataclasses import dataclass +from typing import Any + +from analysis.client import MAX_SUBMITTED_IDENTIFIERS +from util.markdown import escape + +MIN_IDENTIFIERS = 2 +TOP_PATHWAYS = 10 +#: Below this many matched identifiers, say the FDRs rest on very few hits. +SMALL_LIST = 10 +MAX_UNMATCHED_LISTED = 20 + +_ANALYSIS_TERMS = re.compile( + r"\b(gsea|gsa|reactome\s*gsa|ora|enrichment|enriched" + r"|over[\s-]?representation|over[\s-]?represented" + r"|(pathway|enrichment|gene[\s-]*set)\s+analy[sz]\w*" + r"|analy[sz](e|is|ing)\s+(on\s+|of\s+|for\s+|with\s+)?" + r"(my|our|these|this|the\s+following|a|the)?\s*" + r"(gene|genes|list|proteins?|identifiers?|ids)\b)", + re.IGNORECASE, +) +_WANTS_TO_DO_IT = re.compile( + r"\b(run|do|perform|start|carry\s+out|execute|submit|analy[sz]e" + r"|can\s+(we|i|you)|could\s+(we|i|you)|please|want\s+to|would\s+like\s+to" + r"|help\s+me|with\s+(the\s+)?(genes?|proteins?|list)|for\s+(the\s+)?(genes?|proteins?))\b", + re.IGNORECASE, +) + +#: Split on anything an identifier cannot contain. Hyphens stay (isoforms +#: like P04637-2, symbols like HLA-A); dots split (Ensembl versions). +_SEPARATORS = re.compile(r"[^A-Za-z0-9_-]+") +_UNIPROT = re.compile( + r"\A([OPQ][0-9][A-Z0-9]{3}[0-9]|[A-NR-Z][0-9]([A-Z][A-Z0-9]{2}[0-9]){1,2})(-\d+)?\Z" +) +_ENSEMBL = re.compile(r"\AENS[A-Z]*[GTP]\d{11}\Z") +#: A symbol in capitals: EGFR, KRAS, HLA-A. Needs no digit. +_CAPS_SYMBOL = re.compile(r"\A[A-Z][A-Z0-9-]{2,14}\Z") +#: Any case, but with a digit: TP53, Trp53, p53, ERBB2. +_DIGIT_SYMBOL = re.compile(r"\A[A-Za-z][A-Za-z0-9-]{1,14}\Z") + +#: Capitals that are words, not genes, in messages about analyses. +_NOT_GENES = frozenset( + """GSA GSEA ORA GSVA PADOG CAMERA RNA DNA MRNA CDNA RNA-SEQ TSV CSV TXT XLS + XLSX PDF FDR API URL IDS AND THE FOR YOU CAN PLEASE RUN WITH GENE GENES + LIST HELP HOW WHAT WHY USA NIH OICR EBI NCBI HGNC UNIPROT ENSEMBL REACTOME + KEGG OK HELLO THANKS""".split() +) + + +def _looks_like_identifier(token: str, *, shouting: bool) -> bool: + if token.upper() in _NOT_GENES: + return False + if _UNIPROT.match(token) or _ENSEMBL.match(token): + return True + has_digit = any(c.isdigit() for c in token) + if has_digit: + return bool(_DIGIT_SYMBOL.match(token)) + # A message typed in capitals makes every word look like a symbol. + return not shouting and bool(_CAPS_SYMBOL.match(token)) + + +def _is_shouting(text: str) -> bool: + words = re.findall(r"[A-Za-z]{2,}", text) + upper = sum(w.isupper() for w in words) + return len(words) >= 4 and upper / len(words) > 0.5 + + +def identifiers_in(text: str) -> list[str]: + """Identifier-shaped tokens, in order, each once (case-insensitively).""" + shouting = _is_shouting(text) + seen: set[str] = set() + found: list[str] = [] + for token in _SEPARATORS.split(text): + token = token.strip("-") + if ( + token + and token.upper() not in seen + and _looks_like_identifier(token, shouting=shouting) + ): + seen.add(token.upper()) + found.append(token) + return found + + +def gene_list_request(text: str) -> list[str] | None: + """The identifiers to analyse, if this message asks for an analysis of them.""" + if not (_ANALYSIS_TERMS.search(text) and _WANTS_TO_DO_IT.search(text)): + return None + found = identifiers_in(text) + return found if len(found) >= MIN_IDENTIFIERS else None + + +@dataclass(frozen=True) +class Overrepresentation: + """What the reader is told, and whether there was anything to continue.""" + + text: str + #: False when nothing matched -- nothing for a follow-up to be about. + has_pathways: bool + + +def _fdr(value: Any) -> str: + return f"{value:.2g}" if isinstance(value, int | float) else "–" + + +def describe_overrepresentation( + submitted: list[str], + result: dict[str, Any], + browser_url: str, + unmatched: list[str] | None, + *, + truncated: bool = False, +) -> Overrepresentation: + """The reply to a gene list: which matched, the top pathways, a link. + + Also becomes the model's previous turn, so a follow-up ("which of these + involve TP53?") is answered from this result. That is why the table + carries stable identifiers as well as names. + """ + not_found = result.get("identifiersNotFound") + matched = len(submitted) - not_found if isinstance(not_found, int) else None + pathways = [p for p in result.get("pathways") or [] if isinstance(p, dict)] + total = result.get("pathwaysFound") + + lines = [ + "With a list of genes, the Reactome analysis to run is " + "**over-representation**: which pathways contain more of your genes " + "than chance would put there. (A gene set analysis with ReactomeGSA " + "needs expression measurements for each sample β€” attach a matrix " + "with πŸ“Ž if you have one.)", + "", + ] + if matched is not None: + lines.append(f"Matched **{matched} of {len(submitted)}** identifiers.") + if truncated: + lines.append( + f"Only the first {MAX_SUBMITTED_IDENTIFIERS:,} identifiers were submitted." + ) + + if not pathways: + lines += ["", "No Reactome pathways contain any of them."] + else: + count = f"{total:,}" if isinstance(total, int) else str(len(pathways)) + lines += [ + "", + f"Top {len(pathways[:TOP_PATHWAYS])} of {count} pathways, by FDR:", + "", + "| Pathway | Entities found | FDR |", + "|---|---|---|", + ] + for p in pathways[:TOP_PATHWAYS]: + entities = p.get("entities") or {} + name = escape(str(p.get("name", ""))) + st_id = escape(str(p.get("stId", ""))) + lines.append( + f"| {name} ({st_id}) | {entities.get('found', '–')} of " + f"{entities.get('total', '–')} | {_fdr(entities.get('fdr'))} |" + ) + lines += [ + "", + # Measured: TP53, ERBB2, RUNX2 found "7 of 1574" -- entities count + # each form of a protein, so a column called "genes" read as 7 of 3. + "Entities are Reactome's molecules, so one gene can count more " + "than once (its protein in several forms or complexes).", + "", + f"[Open the full result in the Pathway Browser]({browser_url})", + ] + if matched is not None and matched < SMALL_LIST: + lines += [ + "", + f"With {matched} matched genes, each pathway rests on very few " + "of your genes, so treat these as pointers rather than findings.", + ] + + if unmatched: + shown = ", ".join(escape(u) for u in unmatched[:MAX_UNMATCHED_LISTED]) + more = len(unmatched) - MAX_UNMATCHED_LISTED + lines += [ + "", + f"Not found in Reactome: {shown}" + + (f" and {more} more" if more > 0 else ""), + ] + return Overrepresentation(text="\n".join(lines), has_pathways=bool(pathways)) + + +FAILED = ( + "I tried to run an over-representation analysis on those genes, but " + "Reactome's Analysis Service didn't return a result. Please try again in " + "a moment." +) diff --git a/src/gsa/chat.py b/src/gsa/chat.py index 47a3ab3..ecfe3ab 100644 --- a/src/gsa/chat.py +++ b/src/gsa/chat.py @@ -19,6 +19,7 @@ from gsa.client import AnalysisStatus from gsa.job import Finished from gsa.upload import Matrix +from util.markdown import escape #: Shown when a matrix arrives, before anything is submitted. MAX_SAMPLES_LISTED = 24 @@ -131,18 +132,6 @@ def describe_progress(status: AnalysisStatus) -> str: return f"Running the analysis β€” {detail}" -#: Characters with meaning in markdown. Reactome pathway names contain some -#: of them -- measured over a real 2,679-pathway result: `H139Hfs13* PPM1K -#: causes a mild variant of MSUD`, `NOTCH1:M1580_K2555`. Two of either in a -#: cell become emphasis, and a variant identifier like `M1580_K2555` renders -#: as `M1580K2555` with nothing to show it changed. -_MARKDOWN_SPECIAL = "\\`*_[]<>|" - - -def _escape(text: str) -> str: - return "".join(f"\\{ch}" if ch in _MARKDOWN_SPECIAL else ch for ch in text) - - def describe_result(finished: Finished) -> str: """What the person reads. Never a prompt. @@ -166,7 +155,7 @@ def describe_result(finished: Finished) -> str: ] for pathway in top[:10]: lines.append( - f"| {_escape(pathway['name'])} | {pathway['direction']} | {pathway['fdr']:.2g} |" + f"| {escape(pathway['name'])} | {pathway['direction']} | {pathway['fdr']:.2g} |" ) lines.append("") @@ -216,14 +205,12 @@ def asks_to_run_gsa(text: str) -> bool: return bool(_ANALYSIS_TERMS.search(text) and _WANTS_TO_DO_IT.search(text)) -#: The gene-list line points at the website, not at this chat. An earlier -#: draft said "paste the gene list and ask me to analyse it"; checked in a -#: browser, the chat does not run that analysis -- it sends the reader to the -#: website -- so the promise would have been false. +#: The gene-list line promises what `analysis.gene_list` does. It first +#: pointed at the website, because the chat did not run that analysis yet. HOW_TO_RUN_GSA = """Yes β€” you can run a gene set analysis right here in the chat, using ReactomeGSA. 1. **Attach your expression matrix** with the πŸ“Ž button below: a `.tsv` or `.csv` file with genes (or proteins) as rows, samples as columns, and a first row naming the samples. Up to 20 MB. 2. **Tell me which group each sample is in** when I ask β€” for example `control, control, treated, treated`. 3. I'll run the analysis and give you the most significant pathways, a link to view the result in Reactome's Pathway Browser, and the full results table to download. It usually takes a few minutes. -If you only have a **list of genes** rather than measurements for each sample, that's an over-representation analysis instead. You can run it on Reactome's [Analyse Data](https://reactome.org/PathwayBrowser/#TOOL=AT) page by pasting the list.""" +If you only have a **list of genes** rather than measurements for each sample, that's an over-representation analysis instead, and I can run it here too: ask me to analyse them and include the genes in your message β€” for example *run a pathway analysis on TP53, ERBB2, RUNX2*.""" diff --git a/src/util/markdown.py b/src/util/markdown.py new file mode 100644 index 0000000..5991105 --- /dev/null +++ b/src/util/markdown.py @@ -0,0 +1,13 @@ +"""Escaping for text placed into markdown the chat renders. + +Reactome pathway names contain markdown syntax -- measured over a real +2,679-pathway result: `NOTCH1:M1580_K2555`, `H139Hfs13* PPM1K ...`. Two of +`_` or `*` in one table cell become emphasis, and a variant identifier +renders with characters silently missing; a `|` splits the cell. +""" + +SPECIAL = "\\`*_[]<>|" + + +def escape(text: str) -> str: + return "".join(f"\\{ch}" if ch in SPECIAL else ch for ch in text) diff --git a/tests/analysis/test_gene_list.py b/tests/analysis/test_gene_list.py new file mode 100644 index 0000000..735006a --- /dev/null +++ b/tests/analysis/test_gene_list.py @@ -0,0 +1,226 @@ +"""A gene list typed into the chat: recognised, submitted, described. + +The phrasing sets are sized rather than a couple of examples, because the +recogniser is a heuristic and a heuristic's failures are in its edges. +""" + +import asyncio +from collections.abc import Callable + +import httpx +import pytest + +from analysis import client as analysis_client +from analysis.gene_list import ( + describe_overrepresentation, + gene_list_request, + identifiers_in, +) + +# The message that prompted this, verbatim. +ASKED = ( + "can we do a gsa analysis in the chat. I want to do it with genes TP53, " + "ERBB2 and RUNX2" +) + +REQUESTS = [ + (ASKED, ["TP53", "ERBB2", "RUNX2"]), + ("run a pathway analysis on EGFR KRAS BRAF PTEN", ["EGFR", "KRAS", "BRAF", "PTEN"]), + ( + "Please run an enrichment for P04637, Q9Y6K9 and ENSG00000141510", + ["P04637", "Q9Y6K9", "ENSG00000141510"], + ), + ("analyse these genes: tp53 mdm2 cdkn1a", ["tp53", "mdm2", "cdkn1a"]), + ( + "Can you do an over-representation analysis with BRCA1, BRCA2, PALB2?", + ["BRCA1", "BRCA2", "PALB2"], + ), + ("perform ORA on\nTP53\nMDM2\nCDKN1A\n", ["TP53", "MDM2", "CDKN1A"]), + ("run a GSEA with genes MYC, MAX", ["MYC", "MAX"]), + ("I would like to run an enrichment analysis for Trp53, Mdm2", ["Trp53", "Mdm2"]), + ("could you analyse my gene list: HLA-A, HLA-B, B2M", ["HLA-A", "HLA-B", "B2M"]), + ("do a pathway analysis for TP53, TP53, tp53, MDM2", ["TP53", "MDM2"]), + ("run reactome gsa on P04637-2 and Q00987", ["P04637-2", "Q00987"]), + ( + "Please do a gene set analysis for SMAD2, SMAD3, SMAD4, TGFB1", + ["SMAD2", "SMAD3", "SMAD4", "TGFB1"], + ), +] + +NOT_REQUESTS = [ + "can we run gsa in this chat please", # no genes: how-to reply + "what is GSEA?", + "can you explain how TP53 and MDM2 interact?", # genes, no analysis term + "What pathways are BRCA1 and BRCA2 in?", + "Compare GSEA and PADOG analysis methods", # capitals, not genes + "CAN YOU RUN A GSA ANALYSIS ON MY DATA PLEASE", # shouting + "Is TP53 enriched in apoptosis?", # one gene is not a list + "run a pathway analysis on TP53", + "How do I run GSA on RNA-seq data from a TSV or CSV?", + "what does an FDR mean in an ORA result?", + "Tell me about the role of CDK5 in neurons", + "what is the difference between ORA and GSEA", + "", +] + + +@pytest.mark.parametrize(("text", "expected"), REQUESTS) +def test_a_request_with_genes_is_recognised(text: str, expected: list[str]) -> None: + assert gene_list_request(text) == expected + + +@pytest.mark.parametrize("text", NOT_REQUESTS) +def test_other_messages_are_left_to_the_model(text: str) -> None: + assert gene_list_request(text) is None + + +def test_the_words_around_the_genes_are_not_submitted() -> None: + assert identifiers_in(ASKED) == ["TP53", "ERBB2", "RUNX2"] + + +# Shaped like the reply measured from beta on 2026-09-25 for TP53, ERBB2, +# RUNX2, NOTAGENE1 (pathways trimmed). +MEASURED = { + "summary": {"token": "MjAyNjA5MjUxOTM5MzNfMTY%3D", "type": "OVERREPRESENTATION"}, + "identifiersNotFound": 1, + "pathwaysFound": 188, + "pathways": [ + { + "stId": "R-HSA-6804754", + "name": "Regulation of TP53 Expression", + "entities": {"total": 4, "found": 2, "pValue": 2.18e-06, "fdr": 0.000233}, + }, + { + "stId": "R-HSA-0000001", + "name": "NOTCH1:M1580_K2555 | *odd* name", + "entities": {"total": 90, "found": 1, "pValue": 0.01, "fdr": 0.04}, + }, + ], +} +SUBMITTED = ["TP53", "ERBB2", "RUNX2", "NOTAGENE1"] +URL = "https://beta.reactome.org/PathwayBrowser/#/DTAB=AN&ANALYSIS=MjAy" + + +def test_the_reply_reports_matches_pathways_and_the_link() -> None: + reply = describe_overrepresentation(SUBMITTED, MEASURED, URL, ["NOTAGENE1"]) + assert reply.has_pathways + assert "over-representation" in reply.text + assert "Matched **3 of 4** identifiers" in reply.text + assert "Top 2 of 188 pathways" in reply.text + assert ( + "| Regulation of TP53 Expression (R-HSA-6804754) | 2 of 4 | 0.00023 |" + in reply.text + ) + assert URL in reply.text + assert "Not found in Reactome: NOTAGENE1" in reply.text + # Three matched genes: the caution is given. + assert "pointers rather than findings" in reply.text + + +def test_pathway_names_cannot_break_the_table() -> None: + reply = describe_overrepresentation(SUBMITTED, MEASURED, URL, None) + row = next(line for line in reply.text.splitlines() if "R-HSA-0000001" in line) + assert row.count("|") - row.count("\\|") == 4 + assert "M1580\\_K2555" in row + assert "\\*odd\\*" in row + + +def test_no_caution_for_a_list_of_useful_size() -> None: + genes = [f"G{i}" for i in range(12)] + reply = describe_overrepresentation( + genes, {**MEASURED, "identifiersNotFound": 0}, URL, None + ) + assert "Matched **12 of 12**" in reply.text + assert "pointers rather than findings" not in reply.text + + +def test_nothing_matched_says_so_and_offers_nothing_to_continue() -> None: + empty = { + "summary": {"token": "x"}, + "identifiersNotFound": 2, + "pathwaysFound": 0, + "pathways": [], + } + reply = describe_overrepresentation(["AAA1", "BBB2"], empty, URL, ["AAA1", "BBB2"]) + assert not reply.has_pathways + assert "Matched **0 of 2**" in reply.text + assert "No Reactome pathways" in reply.text + assert URL not in reply.text + + +# --- submission, stubbed at the transport ----------------------------------- + + +def _submit( + handler: Callable[[httpx.Request], httpx.Response], ids: list[str] = SUBMITTED +) -> analysis_client.Submitted | None: + async def go() -> analysis_client.Submitted | None: + async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http: + return await analysis_client.submit_identifiers(ids, client=http) + + return asyncio.run(go()) + + +def test_submission_posts_the_list_and_returns_the_token() -> None: + seen: dict[str, object] = {} + + def handler(request: httpx.Request) -> httpx.Response: + seen["path"] = request.url.path + seen["body"] = request.content.decode() + seen["type"] = request.headers["content-type"] + seen["sort"] = request.url.params.get("sortBy") + return httpx.Response(200, json=MEASURED) + + submitted = _submit(handler) + assert submitted is not None + assert submitted.token == MEASURED["summary"]["token"] # type: ignore[index] + assert seen == { + "path": "/AnalysisService/identifiers/projection", + "body": "TP53\nERBB2\nRUNX2\nNOTAGENE1", + "type": "text/plain", + "sort": "ENTITIES_FDR", + } + + +def test_submission_is_bounded() -> None: + sizes: list[int] = [] + + def handler(request: httpx.Request) -> httpx.Response: + sizes.append(len(request.content.decode().split("\n"))) + return httpx.Response(200, json=MEASURED) + + many = [f"G{i}" for i in range(analysis_client.MAX_SUBMITTED_IDENTIFIERS + 50)] + assert _submit(handler, many) is not None + assert sizes == [analysis_client.MAX_SUBMITTED_IDENTIFIERS] + + +@pytest.mark.parametrize( + "response", + [ + httpx.Response(500, text="boom"), + httpx.Response(200, text="not json"), + httpx.Response(200, json={"summary": {}}), + httpx.Response(200, json={"summary": {"token": "../../etc"}}), + httpx.Response(200, json=["a list"]), + ], +) +def test_submission_that_yields_no_usable_token_is_none( + response: httpx.Response, +) -> None: + assert _submit(lambda request: response) is None + + +def test_a_transport_failure_is_none_not_an_exception() -> None: + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectTimeout("slow", request=request) + + assert _submit(handler) is None + + +def test_the_link_is_to_the_service_that_holds_the_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ANALYSIS_BASE_URL", "https://beta.reactome.org/AnalysisService") + assert analysis_client.pathway_browser_url("abc%3D") == ( + "https://beta.reactome.org/PathwayBrowser/#/DTAB=AN&ANALYSIS=abc%3D" + ) diff --git a/tests/gsa/test_gsa_chat.py b/tests/gsa/test_gsa_chat.py index 8280eaa..52e1f11 100644 --- a/tests/gsa/test_gsa_chat.py +++ b/tests/gsa/test_gsa_chat.py @@ -6,6 +6,7 @@ """ import json +import re import tempfile from pathlib import Path @@ -271,8 +272,11 @@ def test_the_reply_says_yes_and_how(self) -> None: assert "20 MB" in chat.HOW_TO_RUN_GSA -def test_the_reply_promises_nothing_the_chat_cannot_do() -> None: - # Verified in a browser: a pasted gene list is NOT analysed in the chat. - # The reply must not say it is. - assert "ask me to analyse it" not in chat.HOW_TO_RUN_GSA - assert "reactome.org/PathwayBrowser/#TOOL=AT" in chat.HOW_TO_RUN_GSA +def test_the_gene_list_example_it_gives_is_one_the_chat_runs() -> None: + # The reply promises the chat analyses a pasted gene list. The example it + # gives must be one the recogniser accepts, or the promise is false. + from analysis.gene_list import gene_list_request + + example = re.search(r"\*(run a pathway analysis on [^*]+)\*", chat.HOW_TO_RUN_GSA) + assert example is not None + assert gene_list_request(example.group(1)) == ["TP53", "ERBB2", "RUNX2"] From d590357ca40129cbef0368fd6f36bd78912b006b Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 25 Sep 2026 20:15:38 +0000 Subject: [PATCH 2/4] Ask before analysing a gene list, and read lists the way people type them An adversarial review of the first version found the recogniser answered 14 of 44 ordinary questions ("Could you explain why IFNG and TNF are enriched...") with a results table, and missed or trimmed 20 of 25 real requests: shouting detection fired on the gene symbols themselves, and lower-case symbols were never read. - The chat now shows the identifiers it read and asks: Run it, or No, answer my question -- which sends the message to the model as before. - Lists are read as lists: after a colon, question mark, newline or preposition, across commas and "and", in any case. - Questions about genes (explain, why, compare, ...) are not requests. - Capitalised words and accession-like strings (NOT, HUMAN, GSE12345, chr17) are not submitted; beta matched NOT as a real identifier. - 'and N more' counts from the service's total, an impossible match count is not shown, odd result shapes do not raise, and a newline in a name cannot end a table row. - Seeding uses the same thread id as the next turn, and logs when it cannot seed. Measured on 35 phrasings written after the rewrite and never tuned against: 15/15 requests read exactly, 0/20 questions fire. Co-Authored-By: Claude Opus 5.5 --- bin/chat-chainlit.py | 68 +++++++-- src/analysis/gene_list.py | 202 ++++++++++++++++++-------- src/util/markdown.py | 6 +- tests/analysis/gene_list_phrases.py | 211 ++++++++++++++++++++++++++++ tests/analysis/test_gene_list.py | 84 ++++++++--- 5 files changed, 480 insertions(+), 91 deletions(-) create mode 100644 tests/analysis/gene_list_phrases.py diff --git a/bin/chat-chainlit.py b/bin/chat-chainlit.py index 6dd265c..67cafbd 100644 --- a/bin/chat-chainlit.py +++ b/bin/chat-chainlit.py @@ -237,7 +237,7 @@ async def continue_from_handoff(handoff_id: str) -> None: 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") + thread_id: str = current_thread_id() data = None try: if isinstance(handoff, AnalysisHandoff): @@ -266,12 +266,53 @@ async def continue_from_handoff(handoff_id: str) -> None: await cl.Message(content=seed.shown_to_reader(handoff)).send() +def current_thread_id() -> str: + """The graph thread for this session -- the same one `main` invokes. + + `on_chat_start` sets it from the session id; a resumed chat may not. + """ + return str(cl.user_session.get("thread_id") or cl.user_session.get("id")) + + +#: How long the Run / No buttons wait before the offer lapses. +PROPOSAL_TIMEOUT_SECONDS = 600 + + +async def confirm_gene_list(identifiers: list[str]) -> bool | None: + """Ask before submitting. True to run, False to answer instead, None on no reply. + + The list is a message of its own because `AskActionMessage` replaces its + content with "Selected: ..." once answered, and the reader should keep + seeing what was submitted. + """ + await cl.Message(content=gene_list.describe_proposal(identifiers)).send() + answer = await cl.AskActionMessage( + content="", + actions=[ + cl.Action(name="gene_list", payload={"run": True}, label="Run it"), + cl.Action( + name="gene_list", + payload={"run": False}, + label="No, answer my question", + ), + ], + timeout=PROPOSAL_TIMEOUT_SECONDS, + ).send() + if answer is None: + return None + return bool((answer.get("payload") or {}).get("run")) + + async def run_gene_list_analysis(text: str, identifiers: list[str]) -> None: """Over-representation on a pasted gene list, and seed it for follow-ups.""" submitted = identifiers[:MAX_SUBMITTED_IDENTIFIERS] + # Up to two service calls; say something is happening meanwhile. + reply_message = cl.Message(content="Running the analysis…") + await reply_message.send() result = await submit_identifiers(submitted) if result is None: - await cl.Message(content=gene_list.FAILED).send() + reply_message.content = gene_list.FAILED + await reply_message.update() return not_found = result.result.get("identifiersNotFound") unmatched = ( @@ -286,7 +327,8 @@ async def run_gene_list_analysis(text: str, identifiers: list[str]) -> None: unmatched, truncated=len(identifiers) > len(submitted), ) - await cl.Message(content=reply.text).send() + reply_message.content = reply.text + await reply_message.update() logger.info( "gene list analysed", extra={"submitted": len(submitted), "has_pathways": reply.has_pathways}, @@ -297,16 +339,18 @@ async def run_gene_list_analysis(text: str, identifiers: list[str]) -> None: # involve TP53?" is answered from this result. The reader typed the list # into this chat, whose every message goes to the model anyway. profile: str = (cl.user_session.get("chat_profile") or "").lower() - thread_id: str = cl.user_session.get("thread_id") or cl.user_session.get("id") try: - await get_graph().seed_history( + seeded = await get_graph().seed_history( profile, - thread_id=thread_id, + thread_id=current_thread_id(), messages=[HumanMessage(content=text), AIMessage(content=reply.text)], ) except Exception: # The reader has their result; only follow-ups lose it. logger.exception("gene list seeding failed") + return + if not seeded: + logger.warning("gene list not seeded", extra={"profile": profile}) @cl.on_window_message @@ -357,10 +401,16 @@ async def main(message: cl.Message) -> None: # A gene list is not a matrix: run what Reactome runs on a list, # over-representation, rather than explaining how to upload a matrix. # Before the GSA check, because the request that prompted this said "gsa". + # It only proposes: the reader sees the identifiers read and chooses, so + # a question that merely looks like a request still gets answered. identifiers = gene_list.gene_list_request(message.content or "") if identifiers is not None: - await run_gene_list_analysis(message.content, identifiers) - return + choice = await confirm_gene_list(identifiers) + if choice is None: + return + if choice: + await run_gene_list_analysis(message.content, identifiers) + return if asks_to_run_gsa(message.content or ""): await cl.Message(content=HOW_TO_RUN_GSA).send() @@ -371,7 +421,7 @@ async def main(message: cl.Message) -> None: chat_profile: str = cl.user_session.get("chat_profile") - thread_id: str = cl.user_session.get("thread_id") + thread_id: str = current_thread_id() chainlit_cb = cl.AsyncLangchainCallbackHandler( stream_final_answer=True, diff --git a/src/analysis/gene_list.py b/src/analysis/gene_list.py index 312ec64..92da57d 100644 --- a/src/analysis/gene_list.py +++ b/src/analysis/gene_list.py @@ -9,16 +9,23 @@ Pure functions, like `gsa.chat`: what counts as a gene list, and what the reader is told, are tested here; the handler is wiring. -**Recognising a request is a heuristic, so it is narrow on purpose.** It -needs all three of: an analysis term, a request to do something, and at -least `MIN_IDENTIFIERS` identifier-shaped tokens. A question *about* genes -("can you explain how TP53 and MDM2 interact?") has no analysis term and -still goes to the model. A single gene is not a list: an enrichment of one -gene is every pathway that gene is in, which the model answers better. +**Recognising a request is a heuristic, and it only proposes.** The chat +shows the identifiers it read and asks before submitting anything, so a +misfire costs one click ("No, answer my question") and a dropped or extra +token is visible before it matters. The first version ran immediately; an +adversarial review found it answered 14 of 44 ordinary questions ("Could you +explain why IFNG and TNF are enriched...") with a results table. + +A message is a request when it has an analysis term, a request to do one, +no sign of being a question *about* something ("explain", "why", +"compare"...), and at least `MIN_IDENTIFIERS` identifiers. A single gene is +not a list: an enrichment of one gene is every pathway it is in, which the +model answers better. """ import re from dataclasses import dataclass +from itertools import pairwise from typing import Any from analysis.client import MAX_SUBMITTED_IDENTIFIERS @@ -29,87 +36,161 @@ #: Below this many matched identifiers, say the FDRs rest on very few hits. SMALL_LIST = 10 MAX_UNMATCHED_LISTED = 20 +#: How many parsed identifiers the confirmation lists by name. +MAX_PROPOSED_LISTED = 40 _ANALYSIS_TERMS = re.compile( - r"\b(gsea|gsa|reactome\s*gsa|ora|enrichment|enriched" - r"|over[\s-]?representation|over[\s-]?represented" - r"|(pathway|enrichment|gene[\s-]*set)\s+analy[sz]\w*" - r"|analy[sz](e|is|ing)\s+(on\s+|of\s+|for\s+|with\s+)?" - r"(my|our|these|this|the\s+following|a|the)?\s*" - r"(gene|genes|list|proteins?|identifiers?|ids)\b)", + r"\b(gsea|gsa|ora|enrich\w*|over[\s-]?represent\w*|analy[sz]\w*" + r"|through\s+reactome|which\s+(\w+\s+)?pathways|to\s+pathways)\b", + re.IGNORECASE, +) +_REQUEST = re.compile( + r"\b(run|perform|carry\s+out|execute|submit|analy[sz]e|map|find" + r"|do\s+(an?\s+|the\s+|my\s+|some\s+)?([\w-]+\s+){0,3}(analysis|enrichment|ora|gsa|gsea)" + r"|which\s+(\w+\s+)?pathways|over[\s-]?represented\s+in" + r"|enrichment\s+(for|on))\b", re.IGNORECASE, ) -_WANTS_TO_DO_IT = re.compile( - r"\b(run|do|perform|start|carry\s+out|execute|submit|analy[sz]e" - r"|can\s+(we|i|you)|could\s+(we|i|you)|please|want\s+to|would\s+like\s+to" - r"|help\s+me|with\s+(the\s+)?(genes?|proteins?|list)|for\s+(the\s+)?(genes?|proteins?))\b", +#: A question about something, not a request to compute it. +_ABOUT = re.compile( + r"\b(explain\w*|why|how|describe|what\s+(does|is|are|would|do)|difference" + r"|compar\w*|check\s+if|whether|interpret\w*|understand|discuss|talk\s+about" + r"|tell\s+me|summar\w*|meaning|mean|literature|correct)\b", re.IGNORECASE, ) -#: Split on anything an identifier cannot contain. Hyphens stay (isoforms -#: like P04637-2, symbols like HLA-A); dots split (Ensembl versions). -_SEPARATORS = re.compile(r"[^A-Za-z0-9_-]+") +#: A token: anything an identifier cannot contain splits. Hyphens stay +#: (isoforms like P04637-2, symbols like HLA-A); dots split (Ensembl versions). +_TOKEN = re.compile(r"[A-Za-z0-9_-]+") _UNIPROT = re.compile( r"\A([OPQ][0-9][A-Z0-9]{3}[0-9]|[A-NR-Z][0-9]([A-Z][A-Z0-9]{2}[0-9]){1,2})(-\d+)?\Z" ) _ENSEMBL = re.compile(r"\AENS[A-Z]*[GTP]\d{11}\Z") #: A symbol in capitals: EGFR, KRAS, HLA-A. Needs no digit. _CAPS_SYMBOL = re.compile(r"\A[A-Z][A-Z0-9-]{2,14}\Z") -#: Any case, but with a digit: TP53, Trp53, p53, ERBB2. +#: Any case, with a digit: TP53, Trp53, p53, ERBB2. _DIGIT_SYMBOL = re.compile(r"\A[A-Za-z][A-Za-z0-9-]{1,14}\Z") +#: Any case, no digit -- only inside a list (see `_list_runs`): egfr, kras. +_ANY_SYMBOL = re.compile(r"\A[A-Za-z][A-Za-z0-9-]{1,14}\Z") +#: Shaped like identifiers, but of something else. +_OTHER_ACCESSIONS = re.compile( + r"\A(chr[0-9XYM]|rs\d|GS[EM]\d|hg\d|GRCh|v\d|pH\d|Q\d\Z|R-[A-Z]{3}-\d" + r"|log\d|COVID|SARS|PMID|HEK\d|MCF\d|HCT\d)", + re.IGNORECASE, +) -#: Capitals that are words, not genes, in messages about analyses. +#: Words, not genes, in messages about analyses -- compared upper-cased. _NOT_GENES = frozenset( - """GSA GSEA ORA GSVA PADOG CAMERA RNA DNA MRNA CDNA RNA-SEQ TSV CSV TXT XLS - XLSX PDF FDR API URL IDS AND THE FOR YOU CAN PLEASE RUN WITH GENE GENES - LIST HELP HOW WHAT WHY USA NIH OICR EBI NCBI HGNC UNIPROT ENSEMBL REACTOME - KEGG OK HELLO THANKS""".split() + """GSA GSEA ORA GSVA PADOG CAMERA DAVID GEO KEGG GO RNA DNA MRNA CDNA + RNA-SEQ SCRNA-SEQ TSV CSV TXT XLS XLSX PDF FDR API URL ID IDS DE DEG DEGS + CHEBI HGNC UNIPROT ENSEMBL REACTOME NCBI EBI NIH OICR USA UK OK HELLO + THANKS HUMAN MOUSE RAT CELLS CELL NOT ALSO AND OR THE FOR YOU CAN PLEASE + RUN WITH GENE GENES LIST HELP HOW WHAT WHY THESE THIS THAT MY OUR ME IT + THEM ON OF IN TO AN IS ARE ALL SOME ANALYSIS ANALYSE ANALYZE ENRICHMENT + PATHWAY PATHWAYS PROTEINS PROTEIN IDENTIFIERS FOLLOWING HERE THANK""".split() ) -def _looks_like_identifier(token: str, *, shouting: bool) -> bool: - if token.upper() in _NOT_GENES: +def _is_identifier(token: str, *, in_list: bool, shouting: bool) -> bool: + if token.upper() in _NOT_GENES or _OTHER_ACCESSIONS.match(token): return False if _UNIPROT.match(token) or _ENSEMBL.match(token): return True - has_digit = any(c.isdigit() for c in token) - if has_digit: + if any(c.isdigit() for c in token): return bool(_DIGIT_SYMBOL.match(token)) - # A message typed in capitals makes every word look like a symbol. + if in_list: + return bool(_ANY_SYMBOL.match(token)) return not shouting and bool(_CAPS_SYMBOL.match(token)) -def _is_shouting(text: str) -> bool: - words = re.findall(r"[A-Za-z]{2,}", text) - upper = sum(w.isupper() for w in words) - return len(words) >= 4 and upper / len(words) > 0.5 +#: Where a list starts: after a colon, a question mark, a newline, or a +#: preposition; and a list continues across commas, semicolons, whitespace +#: and "and"/"or". Anything else -- a full stop, a word that is not an +#: identifier -- ends it. +_LIST_START = re.compile(r"[:?\n]|\b(for|on|of|with|genes|proteins)\b", re.IGNORECASE) +_LIST_GAP = re.compile( + r"\A(\s*[,;]?\s*|\s+(and|or)\s+|\s*,\s*(and|or)\s+)\Z", re.IGNORECASE +) + + +def _list_runs(text: str, shouting: bool) -> set[tuple[int, int]]: + """Spans of tokens inside a list, where lower-case symbols are accepted. + + A list is two or more identifier-shaped tokens in a row, starting after + a list opener. Free text is not a list, so "enrichment for egfr, kras" + reads egfr and kras, and "which pathways are enriched" reads nothing. + """ + spans: set[tuple[int, int]] = set() + for opener in _LIST_START.finditer(text): + run: list[tuple[int, int]] = [] + pos = opener.end() + for match in _TOKEN.finditer(text, pos): + if not _LIST_GAP.match(text[pos : match.start()]): + break + if not _is_identifier(match.group(), in_list=True, shouting=shouting): + break + run.append(match.span()) + pos = match.end() + separators = {text[a:b] for (_, a), (b, _) in pairwise(run)} + # Space-separated lower-case words are prose unless a colon, question + # mark or newline opened the list. + if len(run) >= MIN_IDENTIFIERS and ( + opener.group() in ":?\n" + or any(s.strip() for s in separators) + or "\n" in "".join(separators) + ): + spans.update(run) + return spans -def identifiers_in(text: str) -> list[str]: +def identifiers_in(text: str, *, shouting: bool = False) -> list[str]: """Identifier-shaped tokens, in order, each once (case-insensitively).""" - shouting = _is_shouting(text) + in_list = _list_runs(text, shouting) seen: set[str] = set() found: list[str] = [] - for token in _SEPARATORS.split(text): - token = token.strip("-") - if ( - token - and token.upper() not in seen - and _looks_like_identifier(token, shouting=shouting) - ): + for match in _TOKEN.finditer(text): + token = match.group().strip("-") + if not token or token.upper() in seen: + continue + if _is_identifier(token, in_list=match.span() in in_list, shouting=shouting): seen.add(token.upper()) found.append(token) return found def gene_list_request(text: str) -> list[str] | None: - """The identifiers to analyse, if this message asks for an analysis of them.""" - if not (_ANALYSIS_TERMS.search(text) and _WANTS_TO_DO_IT.search(text)): + """The identifiers to propose analysing, if this message asks for it.""" + request = _REQUEST.search(text) + if not (request and _ANALYSIS_TERMS.search(text)) or _ABOUT.search(text): return None - found = identifiers_in(text) + # Typed in capitals, every word looks like a symbol; then only tokens + # with a digit, or in the accession formats, count. + found = identifiers_in(text, shouting=request.group().isupper()) return found if len(found) >= MIN_IDENTIFIERS else None +def describe_proposal(identifiers: list[str]) -> str: + """What the chat is about to submit, shown before it does.""" + shown = ", ".join(f"`{i}`" for i in identifiers[:MAX_PROPOSED_LISTED]) + more = len(identifiers) - MAX_PROPOSED_LISTED + if more > 0: + shown += f" and {more:,} more" + limit = ( + f" Only the first {MAX_SUBMITTED_IDENTIFIERS:,} will be submitted." + if len(identifiers) > MAX_SUBMITTED_IDENTIFIERS + else "" + ) + return ( + "With a list of genes, the Reactome analysis to run is " + "**over-representation**: which pathways contain more of your genes " + "than chance would put there. (A gene set analysis with ReactomeGSA " + "needs expression measurements for each sample β€” attach a matrix " + "with πŸ“Ž if you have one.)\n\n" + f"I read **{len(identifiers)} identifiers** in your message: {shown}.{limit}\n\n" + "Run the analysis on these?" + ) + + @dataclass(frozen=True) class Overrepresentation: """What the reader is told, and whether there was anything to continue.""" @@ -138,20 +219,19 @@ def describe_overrepresentation( carries stable identifiers as well as names. """ not_found = result.get("identifiersNotFound") - matched = len(submitted) - not_found if isinstance(not_found, int) else None + # Clamped, and hidden if the service counts more misses than we sent: + # a count that cannot be right is not shown. + matched = ( + len(submitted) - not_found + if isinstance(not_found, int) and 0 <= not_found <= len(submitted) + else None + ) pathways = [p for p in result.get("pathways") or [] if isinstance(p, dict)] total = result.get("pathwaysFound") - lines = [ - "With a list of genes, the Reactome analysis to run is " - "**over-representation**: which pathways contain more of your genes " - "than chance would put there. (A gene set analysis with ReactomeGSA " - "needs expression measurements for each sample β€” attach a matrix " - "with πŸ“Ž if you have one.)", - "", - ] + lines = ["**Over-representation analysis** of your gene list, in Reactome."] if matched is not None: - lines.append(f"Matched **{matched} of {len(submitted)}** identifiers.") + lines += ["", f"Matched **{matched} of {len(submitted)}** identifiers."] if truncated: lines.append( f"Only the first {MAX_SUBMITTED_IDENTIFIERS:,} identifiers were submitted." @@ -169,7 +249,9 @@ def describe_overrepresentation( "|---|---|---|", ] for p in pathways[:TOP_PATHWAYS]: - entities = p.get("entities") or {} + entities = p.get("entities") + if not isinstance(entities, dict): + entities = {} name = escape(str(p.get("name", ""))) st_id = escape(str(p.get("stId", ""))) lines.append( @@ -194,7 +276,11 @@ def describe_overrepresentation( if unmatched: shown = ", ".join(escape(u) for u in unmatched[:MAX_UNMATCHED_LISTED]) - more = len(unmatched) - MAX_UNMATCHED_LISTED + # The lookup is paged; the service's own count is the real total. + total_unmatched = not_found if isinstance(not_found, int) else len(unmatched) + more = max(total_unmatched, len(unmatched)) - min( + len(unmatched), MAX_UNMATCHED_LISTED + ) lines += [ "", f"Not found in Reactome: {shown}" diff --git a/src/util/markdown.py b/src/util/markdown.py index 5991105..854b05a 100644 --- a/src/util/markdown.py +++ b/src/util/markdown.py @@ -6,8 +6,10 @@ renders with characters silently missing; a `|` splits the cell. """ -SPECIAL = "\\`*_[]<>|" +SPECIAL = "\\`*_[]<>|~" def escape(text: str) -> str: - return "".join(f"\\{ch}" if ch in SPECIAL else ch for ch in text) + """Literal text, on one line: a newline would end a table row.""" + flat = " ".join(text.splitlines()) + return "".join(f"\\{ch}" if ch in SPECIAL else ch for ch in flat) diff --git a/tests/analysis/gene_list_phrases.py b/tests/analysis/gene_list_phrases.py new file mode 100644 index 0000000..4229f6f --- /dev/null +++ b/tests/analysis/gene_list_phrases.py @@ -0,0 +1,211 @@ +"""Phrasings for the gene-list recogniser, sized, and where each came from. + +- REVIEW_*: written by an adversarial review of the first version, which + fired on 14 of the 44 questions and missed or trimmed 20 of the 25 + requests. The recogniser was then rewritten against these. +- HELD_OUT_*: written after that rewrite and never tuned against. They are + the honest measure: 15/15 and 0/20 when added. + +A positive is exact: the identifiers read, in order. A request that reads +the wrong list is a failure even if it fires. +""" + +REVIEW_REQUESTS: list[tuple[str, list[str]]] = [ + ("run ORA on EGFR KRAS BRAF PTEN NRAS", ["EGFR", "KRAS", "BRAF", "PTEN", "NRAS"]), + ( + "run a pathway analysis on EGFR, KRAS, BRAF, PTEN, NRAS, PIK3CA", + ["EGFR", "KRAS", "BRAF", "PTEN", "NRAS", "PIK3CA"], + ), + ( + "Please run an enrichment on:\nEGFR\nKRAS\nBRAF\nPTEN\nNRAS\nAKT1", + ["EGFR", "KRAS", "BRAF", "PTEN", "NRAS", "AKT1"], + ), + ("Analyse these:\nTP53\nMDM2\nCDKN1A", ["TP53", "MDM2", "CDKN1A"]), + ( + "Can you analyse these genes?\negfr\nkras\nbraf\npten", + ["egfr", "kras", "braf", "pten"], + ), + ("run pathway analysis: egfr, kras, braf, pten", ["egfr", "kras", "braf", "pten"]), + ( + "which pathways are my genes in? TP53, MDM2, CDKN1A, BAX", + ["TP53", "MDM2", "CDKN1A", "BAX"], + ), + ( + "What pathways are over-represented in this list: TP53, MDM2, CDKN1A", + ["TP53", "MDM2", "CDKN1A"], + ), + ( + "Here is my gene list, run it through Reactome: TP53 MDM2 CDKN1A BAX", + ["TP53", "MDM2", "CDKN1A", "BAX"], + ), + ( + "TP53, MDM2, CDKN1A, BAX -- run an enrichment please", + ["TP53", "MDM2", "CDKN1A", "BAX"], + ), + ("Find enriched pathways for TP53, MDM2, CDKN1A", ["TP53", "MDM2", "CDKN1A"]), + ("analyze TP53, MDM2, CDKN1A", ["TP53", "MDM2", "CDKN1A"]), + ( + "Can you run these through the Reactome analysis tool: TP53, MDM2, BAX", + ["TP53", "MDM2", "BAX"], + ), + ( + "run ORA on my list: IL6 TNF IL1B CXCL8 CCL2", + ["IL6", "TNF", "IL1B", "CXCL8", "CCL2"], + ), + ( + "Please analyse the following proteins: P04637, Q00987, P38936", + ["P04637", "Q00987", "P38936"], + ), + ("do an analysis of these genes: egfr kras braf", ["egfr", "kras", "braf"]), + ( + "I have a list of DE genes: MYC, CCND1, CDK4, E2F1. Which Reactome pathways are enriched?", + ["MYC", "CCND1", "CDK4", "E2F1"], + ), + ("enrichment for egfr, kras, braf, pten please", ["egfr", "kras", "braf", "pten"]), + ( + "can you do pathway enrichment on ENSG00000141510 ENSG00000135679", + ["ENSG00000141510", "ENSG00000135679"], + ), + ("map these genes to pathways: TP53 MDM2 CDKN1A", ["TP53", "MDM2", "CDKN1A"]), + ( + "Run an enrichment on GAPDH, ACTB, TUBB, VIM, KRT8, KRT18", + ["GAPDH", "ACTB", "TUBB", "VIM", "KRT8", "KRT18"], + ), + ( + "run an ORA on CD4 CD8A CD3E CD19 MS4A1 NCAM1 ITGAM", + ["CD4", "CD8A", "CD3E", "CD19", "MS4A1", "NCAM1", "ITGAM"], + ), + ( + "Run an ORA on EGFR, ERBB2, ERBB3, ERBB4, GRB2, SOS1, HRAS", + ["EGFR", "ERBB2", "ERBB3", "ERBB4", "GRB2", "SOS1", "HRAS"], + ), + ( + "Perform a pathway analysis on mouse genes Trp53, Mdm2, Cdkn1a", + ["Trp53", "Mdm2", "Cdkn1a"], + ), + ( + "please run a gene set enrichment on BRCA1 BRCA2 ATM ATR CHEK2", + ["BRCA1", "BRCA2", "ATM", "ATR", "CHEK2"], + ), +] + +HELD_OUT_REQUESTS: list[tuple[str, list[str]]] = [ + ( + "can we do a gsa analysis in the chat. I want to do it with genes TP53, ERBB2 and RUNX2", + ["TP53", "ERBB2", "RUNX2"], + ), + ("run a pathway analysis on TP53, ERBB2, RUNX2", ["TP53", "ERBB2", "RUNX2"]), + ( + "Could you run an over-representation analysis for SOX9, RUNX2, SP7, COL1A1?", + ["SOX9", "RUNX2", "SP7", "COL1A1"], + ), + ( + "Please perform an enrichment on these: NFKB1, RELA, IKBKB, CHUK", + ["NFKB1", "RELA", "IKBKB", "CHUK"], + ), + ( + "I'd like to run ORA with STAT1, STAT2, IRF9, ISG15, MX1", + ["STAT1", "STAT2", "IRF9", "ISG15", "MX1"], + ), + ( + "analyse my genes: pik3ca, akt1, mtor, rps6kb1", + ["pik3ca", "akt1", "mtor", "rps6kb1"], + ), + ( + "run GSEA on\nCDK1\nCCNB1\nPLK1\nAURKA\nBUB1", + ["CDK1", "CCNB1", "PLK1", "AURKA", "BUB1"], + ), + ("Do a Reactome enrichment for P04637 and P38398", ["P04637", "P38398"]), + ( + "find pathways enriched in: SLC2A1, HK2, PFKP, LDHA, PKM", + ["SLC2A1", "HK2", "PFKP", "LDHA", "PKM"], + ), + ( + "which pathways contain these genes? FGF2, FGFR1, FRS2, GRB2", + ["FGF2", "FGFR1", "FRS2", "GRB2"], + ), + ("Run an analysis on insr, irs1, irs2, pik3r1", ["insr", "irs1", "irs2", "pik3r1"]), + ("perform ORA: VEGFA; KDR; FLT1; NRP1", ["VEGFA", "KDR", "FLT1", "NRP1"]), + ( + "please analyze CASP3, CASP8, CASP9 and APAF1", + ["CASP3", "CASP8", "CASP9", "APAF1"], + ), + ( + "Can you run an enrichment analysis with my list of genes: Nanog, Pou5f1, Sox2, Klf4", + ["Nanog", "Pou5f1", "Sox2", "Klf4"], + ), + ( + "submit TLR4, MYD88, TRAF6, IRAK4 for pathway analysis", + ["TLR4", "MYD88", "TRAF6", "IRAK4"], + ), +] + +REVIEW_QUESTIONS: list[str] = [ + "Can you explain the enrichment of TP53 targets in apoptosis pathways?", + "Is BRCA1 enriched in DNA repair pathways compared to BRCA2?", + "Please explain how EGFR and KRAS signalling interact", + "Can you tell me which pathways are enriched in my GSEA results for MYC and E2F targets?", + "What does pathway analysis tell us about TP53 and MDM2?", + "Why is my enrichment analysis showing ERBB2 and GRB7 together?", + "I ran an ORA and got TP53 and CDKN1A at the top, what does that mean?", + "Could you describe the role of SMAD2 and SMAD3 in TGF-beta signalling?", + "Please summarise what is known about BRCA1 and BRCA2", + "Can you do a literature summary on IL6 and STAT3?", + "In a gene set analysis, why would HLA-A and B2M both show up?", + "How does over-representation analysis handle genes like TP53 that are in many pathways?", + "What pathways involve both PTEN and AKT1?", + "Can I use UniProt IDs like P04637 and P38398 in the analysis tool?", + "Is GSEA better than ORA for a list like CD4, CD8A, CD3E?", + "Could you explain why IFNG and TNF are enriched in immune pathways?", + "Please help me understand the interaction between MTOR and RPTOR", + "What does a p<0.05 FDR mean for pathway analysis of NOTCH1 targets?", + "Can you show me the reactions in which CDK1 and CCNB1 take part?", + "I want to understand the enrichment of JAK2 and STAT5A in hematopoiesis", + "Run me through how pathway analysis works for genes like APOE and APP", + "Can you list the pathways where VEGFA and KDR are found?", + "Would like to know if ESR1 and PGR are enriched in breast cancer pathways", + "Help me interpret this: WNT signalling enriched, CTNNB1 and APC found", + "Can you do a comparison of MAPK1 and MAPK3 functions?", + "Please describe the reaction where ATP is hydrolysed by ABCB1", + "How is NF-kB activated by TNF and IL1B?", + "Which genes in the TCA cycle, like CS and IDH2, are enriched in my data?", + "What is the enrichment score for TP53 in the Reactome GSA output?", + "Could we discuss pathway analysis results where SOX2 and POU5F1 appear?", + "I did an enrichment with DAVID for FOXO1 and FOXO3, can you compare with Reactome?", + "Can you explain the difference between ORA and GSEA using BRCA1 and ATM as examples?", + "Please tell me whether KRAS G12D is enriched in pancreatic cancer", + "Can you check if my pathway analysis with PD-1 and CTLA4 is correct?", + "Could you help me with an analysis of how TP53 and MDM2 regulate each other?", + "Is there an enrichment of ERBB2 amplification in HER2+ breast cancer?", + "Can you run through the steps of the insulin pathway: INS, INSR, IRS1?", + "Do HIF1A and VHL appear together in any enriched pathway?", + "What are the downstream effects of mTORC1 and mTORC2 in pathway analysis?", + "Please explain why GAPDH and ACTB are used as housekeeping genes in enrichment", + "Can you describe the ORA method? I have genes like TP53.", + "How many genes are in the pathway enriched for BCL2 and BAX?", + "Can we talk about the 2x enrichment of CD19 and MS4A1 in B cells?", + "What would pathway analysis show for a knockout of IL2 and IL2RA?", +] + +HELD_OUT_QUESTIONS: list[str] = [ + "Which pathways is TP53 in?", + "Can you explain what an ORA does with genes like MYC and MAX?", + "Why does my enrichment show HLA-A and HLA-B at the top?", + "How are RUNX2 and SP7 related in osteoblast differentiation?", + "Is ERBB2 over-represented in breast cancer compared to ERBB3?", + "What is the role of NOTCH1 and JAG1 in development?", + "Tell me about BRCA1 and BRCA2 in homologous recombination", + "Can you summarise the MAPK cascade with BRAF and MEK1?", + "I ran GSEA and CDK1 and PLK1 came up. Does that make sense?", + "Please give me the reactions that involve ATM and CHEK2", + "Could you find the literature on TP53 and MDM2?", + "What does FDR mean in my analysis with EGFR and KRAS?", + "Does CTNNB1 bind APC in the destruction complex?", + "Show me the diagram for the pathway with SMAD2 and SMAD4", + "What analysis can I do with DESeq2 output for 3 conditions?", + "run the pathway browser for me", + "Are IL6 and STAT3 enriched in the JAK-STAT pathway?", + "Is there a pathway that links GLUT1 and HK2?", + "Which of these involve TP53?", + "Can you find pathways where both INS and INSR are present?", +] diff --git a/tests/analysis/test_gene_list.py b/tests/analysis/test_gene_list.py index 735006a..3a76e18 100644 --- a/tests/analysis/test_gene_list.py +++ b/tests/analysis/test_gene_list.py @@ -7,12 +7,15 @@ import asyncio from collections.abc import Callable +import gene_list_phrases as phrases import httpx import pytest from analysis import client as analysis_client from analysis.gene_list import ( + MAX_PROPOSED_LISTED, describe_overrepresentation, + describe_proposal, gene_list_request, identifiers_in, ) @@ -25,42 +28,34 @@ REQUESTS = [ (ASKED, ["TP53", "ERBB2", "RUNX2"]), - ("run a pathway analysis on EGFR KRAS BRAF PTEN", ["EGFR", "KRAS", "BRAF", "PTEN"]), - ( - "Please run an enrichment for P04637, Q9Y6K9 and ENSG00000141510", - ["P04637", "Q9Y6K9", "ENSG00000141510"], - ), - ("analyse these genes: tp53 mdm2 cdkn1a", ["tp53", "mdm2", "cdkn1a"]), - ( - "Can you do an over-representation analysis with BRCA1, BRCA2, PALB2?", - ["BRCA1", "BRCA2", "PALB2"], - ), ("perform ORA on\nTP53\nMDM2\nCDKN1A\n", ["TP53", "MDM2", "CDKN1A"]), ("run a GSEA with genes MYC, MAX", ["MYC", "MAX"]), - ("I would like to run an enrichment analysis for Trp53, Mdm2", ["Trp53", "Mdm2"]), - ("could you analyse my gene list: HLA-A, HLA-B, B2M", ["HLA-A", "HLA-B", "B2M"]), ("do a pathway analysis for TP53, TP53, tp53, MDM2", ["TP53", "MDM2"]), ("run reactome gsa on P04637-2 and Q00987", ["P04637-2", "Q00987"]), + # Words in capitals and accession-like strings are not submitted. ( - "Please do a gene set analysis for SMAD2, SMAD3, SMAD4, TGFB1", - ["SMAD2", "SMAD3", "SMAD4", "TGFB1"], + "run an enrichment on TP53, MDM2 in HUMAN NOT MOUSE, see GSE12345 chr17", + ["TP53", "MDM2"], ), + *phrases.REVIEW_REQUESTS, + *phrases.HELD_OUT_REQUESTS, ] NOT_REQUESTS = [ - "can we run gsa in this chat please", # no genes: how-to reply + "can we run gsa in this chat please", # no genes: the how-to reply "what is GSEA?", - "can you explain how TP53 and MDM2 interact?", # genes, no analysis term - "What pathways are BRCA1 and BRCA2 in?", "Compare GSEA and PADOG analysis methods", # capitals, not genes "CAN YOU RUN A GSA ANALYSIS ON MY DATA PLEASE", # shouting - "Is TP53 enriched in apoptosis?", # one gene is not a list - "run a pathway analysis on TP53", + # Shouting, with capitals the stoplist does not know. + "RUN AN ENRICHMENT ANALYSIS FROM MY EXPERIMENT TODAY", + # Space-separated lower-case words after "for" are prose, not a list. + "run an enrichment for mice given high doses", + "run a pathway analysis on TP53", # one gene is not a list "How do I run GSA on RNA-seq data from a TSV or CSV?", - "what does an FDR mean in an ORA result?", - "Tell me about the role of CDK5 in neurons", "what is the difference between ORA and GSEA", "", + *phrases.REVIEW_QUESTIONS, + *phrases.HELD_OUT_QUESTIONS, ] @@ -74,6 +69,21 @@ def test_other_messages_are_left_to_the_model(text: str) -> None: assert gene_list_request(text) is None +def test_the_sets_are_the_size_they_claim() -> None: + # Sized sets are the point: a pass at n=2 says nothing about a rate. + assert len(REQUESTS) >= 40 + assert len(NOT_REQUESTS) >= 70 + + +def test_the_proposal_lists_what_will_be_submitted() -> None: + proposal = describe_proposal(["TP53", "ERBB2", "RUNX2"]) + assert "over-representation" in proposal + assert "**3 identifiers**" in proposal + assert "`TP53`, `ERBB2`, `RUNX2`" in proposal + many = describe_proposal([f"G{i}" for i in range(MAX_PROPOSED_LISTED + 5)]) + assert "and 5 more" in many + + def test_the_words_around_the_genes_are_not_submitted() -> None: assert identifiers_in(ASKED) == ["TP53", "ERBB2", "RUNX2"] @@ -104,7 +114,7 @@ def test_the_words_around_the_genes_are_not_submitted() -> None: def test_the_reply_reports_matches_pathways_and_the_link() -> None: reply = describe_overrepresentation(SUBMITTED, MEASURED, URL, ["NOTAGENE1"]) assert reply.has_pathways - assert "over-representation" in reply.text + assert "Over-representation analysis" in reply.text assert "Matched **3 of 4** identifiers" in reply.text assert "Top 2 of 188 pathways" in reply.text assert ( @@ -148,6 +158,36 @@ def test_nothing_matched_says_so_and_offers_nothing_to_continue() -> None: assert URL not in reply.text +def test_the_remainder_of_unmatched_counts_all_of_them() -> None: + # The notFound lookup is paged (50); the reply said "and 30 more" for 300. + many = [f"G{i}" for i in range(300)] + result = {**MEASURED, "identifiersNotFound": 300} + reply = describe_overrepresentation(many, result, URL, many[:50]) + assert "and 280 more" in reply.text + + +def test_an_impossible_match_count_is_not_shown() -> None: + result = {**MEASURED, "identifiersNotFound": 5} + reply = describe_overrepresentation(["A1", "B2"], result, URL, None) + assert "Matched" not in reply.text + assert "-3" not in reply.text + + +def test_an_odd_entities_field_does_not_raise() -> None: + odd = {**MEASURED, "pathways": [{"stId": "R-HSA-1", "name": "X", "entities": "?"}]} + reply = describe_overrepresentation(SUBMITTED, odd, URL, None) + assert "| X (R-HSA-1) | – of – | – |" in reply.text + + +def test_a_newline_in_a_name_stays_in_its_row() -> None: + odd = { + **MEASURED, + "pathways": [{"stId": "R-HSA-1", "name": "A\nB", "entities": {}}], + } + reply = describe_overrepresentation(SUBMITTED, odd, URL, None) + assert "| A B (R-HSA-1) |" in reply.text + + # --- submission, stubbed at the transport ----------------------------------- From 2b7559e6b6bcc6b049ee4b51ea17419341f445bc Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 25 Sep 2026 20:51:58 +0000 Subject: [PATCH 3/4] Read gene lists in one pass, and offer the analysis without blocking A second adversarial review found: - quadratic parsing: a pasted 4,000-gene column took 38 s, 40,000 newlines 24 s, on the event loop all sessions share; - 12 of 40 fresh questions still proposed an analysis; - trailing sentences read as genes ("... using default settings"); - AskActionMessage locked the message box until a click, and a timeout dropped the question; - No on a message saying "gsa" offered the declined list again. Now: - One pass over the tokens, with possessive gap patterns and a 60K cap. Timing tests cover each pathological input the review found. - A list keeps one separator throughout; a blank line after a list ends reading; "X and Y" in a question is prose, not a list. - The offer is a message with actions: the reader can click Run, click No, type "yes", or type on. An older offer's buttons still work. - No sends a GSA question to the matrix-only how-to, anything else to the model. Measured on a third set written after this and never tuned against: 11/12 requests read exactly, 1/15 questions offered an analysis. Both failures are pinned as known limits. Co-Authored-By: Claude Opus 5.5 --- bin/chat-chainlit.py | 185 +++++++++++++------- src/analysis/gene_list.py | 255 +++++++++++++++++++++------- src/gsa/chat.py | 12 +- tests/analysis/gene_list_phrases.py | 235 +++++++++++++++++++++++++ tests/analysis/test_gene_list.py | 85 +++++++++- tests/gsa/test_gsa_chat.py | 9 + 6 files changed, 653 insertions(+), 128 deletions(-) diff --git a/bin/chat-chainlit.py b/bin/chat-chainlit.py index 67cafbd..63d0dc1 100644 --- a/bin/chat-chainlit.py +++ b/bin/chat-chainlit.py @@ -3,6 +3,7 @@ # or get_data_layer. Not fixable here; it needs stubs upstream. The file is # named with a hyphen, so it cannot be listed in [[tool.mypy.overrides]]. import os +import uuid from pathlib import Path import chainlit as cl @@ -31,7 +32,7 @@ result_file_kwargs, run_analysis, ) -from gsa.chat import HOW_TO_RUN_GSA, asks_to_run_gsa +from gsa.chat import HOW_TO_RUN_GSA, HOW_TO_RUN_GSA_WITH_A_MATRIX, asks_to_run_gsa from handoff import seed from handoff.store import AnalysisHandoff, handoffs from handoff.window import acknowledgement, claimed_id @@ -274,33 +275,76 @@ def current_thread_id() -> str: return str(cl.user_session.get("thread_id") or cl.user_session.get("id")) -#: How long the Run / No buttons wait before the offer lapses. -PROPOSAL_TIMEOUT_SECONDS = 600 +#: Proposals kept per session, so an older proposal's buttons still work. +MAX_PENDING_PROPOSALS = 5 -async def confirm_gene_list(identifiers: list[str]) -> bool | None: - """Ask before submitting. True to run, False to answer instead, None on no reply. +async def propose_gene_list(message: cl.Message, identifiers: list[str]) -> None: + """Offer to analyse the list; the buttons answer later, without blocking. - The list is a message of its own because `AskActionMessage` replaces its - content with "Selected: ..." once answered, and the reader should keep - seeing what was submitted. + Not `AskActionMessage`: that disables the message box until the reader + clicks, and drops the question if they never do. Here they can click + Run, click No, type "yes", or simply type on. """ - await cl.Message(content=gene_list.describe_proposal(identifiers)).send() - answer = await cl.AskActionMessage( - content="", - actions=[ - cl.Action(name="gene_list", payload={"run": True}, label="Run it"), - cl.Action( - name="gene_list", - payload={"run": False}, - label="No, answer my question", - ), - ], - timeout=PROPOSAL_TIMEOUT_SECONDS, + proposal_id = uuid.uuid4().hex + run = cl.Action(name="gene_list_run", payload={"id": proposal_id}, label="Run it") + decline = cl.Action( + name="gene_list_no", + payload={"id": proposal_id}, + label="No, answer my question", + ) + pending: dict[str, dict] = cl.user_session.get("gene_list_pending") or {} + pending[proposal_id] = { + "text": message.content, + "message_id": message.id, + "identifiers": identifiers, + "actions": [run, decline], + } + while len(pending) > MAX_PENDING_PROPOSALS: + pending.pop(next(iter(pending))) + cl.user_session.set("gene_list_pending", pending) + # A typed "yes" means the proposal just made, and only that one. + cl.user_session.set("gene_list_latest", proposal_id) + await cl.Message( + content=gene_list.describe_proposal(identifiers), actions=[run, decline] ).send() - if answer is None: - return None - return bool((answer.get("payload") or {}).get("run")) + + +async def take_proposal(proposal_id: str | None) -> dict | None: + """Claim a proposal once, and take its buttons away.""" + pending: dict[str, dict] = cl.user_session.get("gene_list_pending") or {} + proposal = pending.pop(proposal_id, None) if proposal_id else None + cl.user_session.set("gene_list_pending", pending) + if cl.user_session.get("gene_list_latest") == proposal_id: + cl.user_session.set("gene_list_latest", None) + if proposal is not None: + for action in proposal["actions"]: + await action.remove() + return proposal + + +@cl.action_callback("gene_list_run") +async def on_gene_list_run(action: cl.Action) -> None: + proposal = await take_proposal(action.payload.get("id")) + if proposal is None: + await cl.Message(content=gene_list.EXPIRED).send() + return + await run_gene_list_analysis(proposal["text"], proposal["identifiers"]) + + +@cl.action_callback("gene_list_no") +async def on_gene_list_no(action: cl.Action) -> None: + proposal = await take_proposal(action.payload.get("id")) + if proposal is None: + return + # Not the full GSA how-to: for "can we do a gsa analysis... TP53, ERBB2" + # it ends by offering to analyse the list just declined. Not the model + # either: grounded in the website's user guide, it says GSA cannot be + # done in this chat -- the bug the how-to exists to fix. Measured both. + if asks_to_run_gsa(proposal["text"]): + await cl.Message(content=HOW_TO_RUN_GSA_WITH_A_MATRIX).send() + return + await answer_with_model(proposal["text"], proposal["message_id"]) async def run_gene_list_analysis(text: str, identifiers: list[str]) -> None: @@ -376,46 +420,8 @@ async def on_window_message(message: object) -> None: await cl.send_window_message(acknowledgement(handoff_id)) -@cl.on_message -async def main(message: cl.Message) -> None: - if await message_rate_limited(config): - return - - await static_messages(config, TriggerEvent.on_message) - - # An attached matrix routes to the analysis flow instead of the graph. - # - # Not a tool the agent calls: the run takes minutes, which is longer - # than a chat turn, and the matrix is over a megabyte, which must never - # enter the model's context. A tool call would put the model in the - # middle of both problems. - attachment = matrix_attachment(getattr(message, "elements", None)) - if attachment is not None: - await run_gsa_analysis(attachment) - return - - # Asked in words rather than by attaching a file. The answer path is - # grounded in the user guide, which describes the website's GSA page, so - # it said this chat could not do it. Answered here instead -- chat only, - # because the same answer path serves the search page, which cannot. - # A gene list is not a matrix: run what Reactome runs on a list, - # over-representation, rather than explaining how to upload a matrix. - # Before the GSA check, because the request that prompted this said "gsa". - # It only proposes: the reader sees the identifiers read and chooses, so - # a question that merely looks like a request still gets answered. - identifiers = gene_list.gene_list_request(message.content or "") - if identifiers is not None: - choice = await confirm_gene_list(identifiers) - if choice is None: - return - if choice: - await run_gene_list_analysis(message.content, identifiers) - return - - if asks_to_run_gsa(message.content or ""): - await cl.Message(content=HOW_TO_RUN_GSA).send() - return - +async def answer_with_model(content: str, message_id: str) -> None: + """The ordinary turn: the graph answers, streamed, with its sources.""" message_count: int = cl.user_session.get("message_count", 0) + 1 cl.user_session.set("message_count", message_count) @@ -431,7 +437,7 @@ async def main(message: cl.Message) -> None: enable_postprocess: bool = is_feature_enabled(config, "postprocessing") result: OutputState = await get_graph().ainvoke( - message.content, + content, chat_profile.lower(), callbacks=[chainlit_cb, openai_cb], thread_id=thread_id, @@ -451,4 +457,53 @@ async def main(message: cl.Message) -> None: await static_messages(config, after_messages=message_count) - save_openai_metrics(message.id, openai_cb) + save_openai_metrics(message_id, openai_cb) + + +@cl.on_message +async def main(message: cl.Message) -> None: + if await message_rate_limited(config): + return + + await static_messages(config, TriggerEvent.on_message) + + # An attached matrix routes to the analysis flow instead of the graph. + # + # Not a tool the agent calls: the run takes minutes, which is longer + # than a chat turn, and the matrix is over a megabyte, which must never + # enter the model's context. A tool call would put the model in the + # middle of both problems. + attachment = matrix_attachment(getattr(message, "elements", None)) + if attachment is not None: + await run_gsa_analysis(attachment) + return + + # "yes" to the proposal just made is the same as clicking Run. Only the + # message straight after it: a "yes" later answers something else. + latest = cl.user_session.get("gene_list_latest") + cl.user_session.set("gene_list_latest", None) + if latest and gene_list.confirms(message.content or ""): + proposal = await take_proposal(latest) + if proposal is not None: + await run_gene_list_analysis(proposal["text"], proposal["identifiers"]) + return + + # A gene list is not a matrix: offer what Reactome runs on a list, + # over-representation, rather than explaining how to upload a matrix. + # Before the GSA check, because the request that prompted this said "gsa". + # It only proposes; a question that merely looks like a request is one + # click from being answered. + identifiers = gene_list.gene_list_request(message.content or "") + if identifiers is not None: + await propose_gene_list(message, identifiers) + return + + # Asked in words rather than by attaching a file. The answer path is + # grounded in the user guide, which describes the website's GSA page, so + # it said this chat could not do it. Answered here instead -- chat only, + # because the same answer path serves the search page, which cannot. + if asks_to_run_gsa(message.content or ""): + await cl.Message(content=HOW_TO_RUN_GSA).send() + return + + await answer_with_model(message.content, message.id) diff --git a/src/analysis/gene_list.py b/src/analysis/gene_list.py index 92da57d..47769a0 100644 --- a/src/analysis/gene_list.py +++ b/src/analysis/gene_list.py @@ -25,7 +25,6 @@ import re from dataclasses import dataclass -from itertools import pairwise from typing import Any from analysis.client import MAX_SUBMITTED_IDENTIFIERS @@ -48,14 +47,19 @@ r"\b(run|perform|carry\s+out|execute|submit|analy[sz]e|map|find" r"|do\s+(an?\s+|the\s+|my\s+|some\s+)?([\w-]+\s+){0,3}(analysis|enrichment|ora|gsa|gsea)" r"|which\s+(\w+\s+)?pathways|over[\s-]?represented\s+in" - r"|enrichment\s+(for|on))\b", + r"|enrichment\s+(for|on)|(enrichment|ora|gsea|analysis)\s+please)\b", re.IGNORECASE, ) #: A question about something, not a request to compute it. _ABOUT = re.compile( r"\b(explain\w*|why|how|describe|what\s+(does|is|are|would|do)|difference" r"|compar\w*|check\s+if|whether|interpret\w*|understand|discuss|talk\s+about" - r"|tell\s+me|summar\w*|meaning|mean|literature|correct)\b", + r"|tell\s+me|summar\w*|meaning|mean|literature|correct" + # From a second review's fresh questions: prose about genes, not a list. + r"|role|relationship|between|shared|(? bool: - if token.upper() in _NOT_GENES or _OTHER_ACCESSIONS.match(token): - return False - if _UNIPROT.match(token) or _ENSEMBL.match(token): - return True - if any(c.isdigit() for c in token): - return bool(_DIGIT_SYMBOL.match(token)) - if in_list: - return bool(_ANY_SYMBOL.match(token)) - return not shouting and bool(_CAPS_SYMBOL.match(token)) - - -#: Where a list starts: after a colon, a question mark, a newline, or a -#: preposition; and a list continues across commas, semicolons, whitespace -#: and "and"/"or". Anything else -- a full stop, a word that is not an -#: identifier -- ends it. -_LIST_START = re.compile(r"[:?\n]|\b(for|on|of|with|genes|proteins)\b", re.IGNORECASE) -_LIST_GAP = re.compile( - r"\A(\s*[,;]?\s*|\s+(and|or)\s+|\s*,\s*(and|or)\s+)\Z", re.IGNORECASE +#: Longer than any list worth typing (3,000 identifiers of ~15 characters is +#: 48K). A bound on the work, not a policy: an attached file is the way in +#: for more. +MAX_MESSAGE_CHARS = 60_000 + +#: Words a list follows: "for", "on", "genes", and the request verbs +#: themselves ("analyze TP53, MDM2"). +_OPENER_WORDS = frozenset( + """FOR ON OF WITH GENES PROTEINS LIST THESE FOLLOWING RUN PERFORM SUBMIT + ANALYSE ANALYZE MAP FIND""".split() +) +#: Joining words, read as part of the gap between two tokens. +_JOINERS = frozenset({"and", "or"}) +_REQUEST_START = re.compile( + r"\A\s*(please\s+)?(run|perform|do|analy[sz]e|submit|execute|map|find)\b", + re.IGNORECASE, +) +#: List markers people paste: "1. TP53", "2) MDM2", "- CDKN1A", "β€’ BAX". +_MARKERS = re.compile(r"(?m)^[ \t]*(?:\d{1,4}[.)]|[-*β€’])[ \t]+") +_ENSEMBL_VERSION = re.compile(r"\b(ENS[A-Z]*[GTP]\d{11})\.\d+\b") +# Possessive quantifiers throughout: a gap can be 40,000 newlines long, and +# `\s*,?\s*` over that backtracks quadratically (measured: 24 s). +_AND = re.compile(r"\A\s*+,?\s*+(and|or)\s++\Z", re.IGNORECASE) +_COMMA = re.compile(r"\A[ \t]*+[,;\t|/][ \t]*+\Z") +_SPACE = re.compile(r"\A[ \t]++\Z") +_NEWLINE = re.compile(r"\A[ \t]*+[,;]?[ \t]*+\n[ \t]*+\Z") +_BLANK_LINE = re.compile(r"\n[ \t]*+\n") +#: Pasted text arrives with Windows line ends, non-breaking spaces, quotes +#: and full-width commas; each would otherwise end a list. +_NORMALISE = str.maketrans( + { + "\r": "\n", + "\u00a0": " ", + "\uff0c": ",", + "\u3001": ",", + '"': " ", + "'": " ", + "\u201c": " ", + "\u201d": " ", + "\u2018": " ", + "\u2019": " ", + "`": " ", + } ) -def _list_runs(text: str, shouting: bool) -> set[tuple[int, int]]: - """Spans of tokens inside a list, where lower-case symbols are accepted. +def _gap_kind(gap: str) -> str | None: + """How two neighbouring tokens are joined; None ends a list. - A list is two or more identifier-shaped tokens in a row, starting after - a list opener. Free text is not a list, so "enrichment for egfr, kras" - reads egfr and kras, and "which pathways are enriched" reads nothing. + Plain regexes over a gap that is itself bounded by two tokens, so the + whole message is scanned once. The first version walked from every + opener to the end of the message: a pasted column of 4,000 genes took + 38 s, on the event loop every session shares. """ - spans: set[tuple[int, int]] = set() - for opener in _LIST_START.finditer(text): - run: list[tuple[int, int]] = [] - pos = opener.end() - for match in _TOKEN.finditer(text, pos): - if not _LIST_GAP.match(text[pos : match.start()]): - break - if not _is_identifier(match.group(), in_list=True, shouting=shouting): + if _COMMA.match(gap): + return "comma" + if _NEWLINE.match(gap): + return "newline" + if _AND.match(gap): + return "and" + if _SPACE.match(gap): + return "space" + return None + + +def _strength(token: str) -> str | None: + """ "strong" (a digit or an accession format), "caps", "weak", or None.""" + if token.upper() in _NOT_GENES or _OTHER_ACCESSIONS.match(token): + return None + if _UNIPROT.match(token) or _ENSEMBL.match(token): + return "strong" + if any(c.isdigit() for c in token): + return "strong" if _DIGIT_SYMBOL.match(token) else None + if _CAPS_SYMBOL.match(token): + return "caps" + # "down-regulated" is a word; HLA-A, in capitals, was caught above. + return "weak" if _ANY_SYMBOL.match(token) and "-" not in token else None + + +@dataclass +class _Run: + opened_by_mark: bool + tokens: list[tuple[str, str]] # (token, strength) + kind: str | None = None + ended_by_and: bool = False + + +def _accepted(run: _Run, *, shouting: bool, pair_ok: bool) -> list[str]: + tokens = run.tokens + # Space-separated plain words are prose unless a colon, question mark or + # newline announced a list: "on TP53 MDM2 using default settings" reads + # TP53 and MDM2. Capitals are words too when the whole message is. + if not run.opened_by_mark: + for index, (_, strength) in enumerate(tokens): + if (strength == "weak" and run.kind in (None, "space", "and")) or ( + strength == "caps" and shouting + ): + tokens = tokens[:index] break - run.append(match.span()) - pos = match.end() - separators = {text[a:b] for (_, a), (b, _) in pairwise(run)} - # Space-separated lower-case words are prose unless a colon, question - # mark or newline opened the list. - if len(run) >= MIN_IDENTIFIERS and ( - opener.group() in ":?\n" - or any(s.strip() for s in separators) - or "\n" in "".join(separators) - ): - spans.update(run) - return spans + if len(tokens) < MIN_IDENTIFIERS: + return [] + # "X and Y" is how prose names two genes; a list says it with commas. + if len(tokens) == 2 and run.kind == "and" and not pair_ok: + return [] + return [token for token, _ in tokens] def identifiers_in(text: str, *, shouting: bool = False) -> list[str]: - """Identifier-shaped tokens, in order, each once (case-insensitively).""" - in_list = _list_runs(text, shouting) - seen: set[str] = set() + """The identifiers in the lists in a message, in order, each once. + + A list is two or more identifier-shaped tokens joined the same way + throughout -- commas, newlines, tabs or spaces, with "and" before the + last -- and it starts at the beginning of the message, after a colon, + question mark or newline, or after "for", "on", "genes"... A change of + separator, a blank line, or a word that is not an identifier ends it, + so a trailing sentence is not read as genes. + """ + text = text.replace("\r\n", "\n").translate(_NORMALISE) + text = _ENSEMBL_VERSION.sub(r"\1", _MARKERS.sub("", text)) + pair_ok = "?" not in text and bool(_REQUEST_START.match(text)) found: list[str] = [] + run: _Run | None = None + previous_end = 0 + previous_token = "" + + def close() -> None: + if run is not None: + found.extend(_accepted(run, shouting=shouting, pair_ok=pair_ok)) + for match in _TOKEN.finditer(text): token = match.group().strip("-") - if not token or token.upper() in seen: - continue - if _is_identifier(token, in_list=match.span() in in_list, shouting=shouting): + if token.lower() in _JOINERS: + continue # stays in the gap: "TP53, ERBB2 and RUNX2" + gap = text[previous_end : match.start()] + previous_end = match.end() + if _BLANK_LINE.search(gap): + close() + run = None + if found: + # A blank line after a list: what follows is a new paragraph + # -- "Best,\nJohn" -- not more genes. + break + strength = _strength(token) if token else None + kind = _gap_kind(gap) + # One separator throughout; "and" may join the last one, after which + # the list is over. + if ( + run is not None + and not run.ended_by_and + and strength is not None + and kind is not None + and (run.kind is None or kind in (run.kind, "and")) + ): + run.tokens.append((token, strength)) + run.kind = run.kind or kind + run.ended_by_and = kind == "and" + else: + close() + run = None + marked = any(c in gap for c in ":?\n") + opens = ( + match.start() == 0 or marked or previous_token.upper() in _OPENER_WORDS + ) + if strength is not None and opens: + run = _Run(opened_by_mark=marked, tokens=[(token, strength)]) + previous_token = token + close() + + seen: set[str] = set() + unique: list[str] = [] + for token in found: + if token.upper() not in seen: seen.add(token.upper()) - found.append(token) - return found + unique.append(token) + return unique def gene_list_request(text: str) -> list[str] | None: """The identifiers to propose analysing, if this message asks for it.""" + if len(text) > MAX_MESSAGE_CHARS: + return None request = _REQUEST.search(text) if not (request and _ANALYSIS_TERMS.search(text)) or _ABOUT.search(text): return None @@ -187,10 +306,22 @@ def describe_proposal(identifiers: list[str]) -> str: "needs expression measurements for each sample β€” attach a matrix " "with πŸ“Ž if you have one.)\n\n" f"I read **{len(identifiers)} identifiers** in your message: {shown}.{limit}\n\n" - "Run the analysis on these?" + "Run the analysis on these? (Or just type *yes*.)" ) +_CONFIRMS = re.compile( + r"\A\s*(yes|y|yep|yeah|ok|okay|sure|go|go\s+ahead|run|run\s+it|do\s+it" + r"|please|please\s+do|yes,?\s+please|please\s+run\s+it)\s*[.!]*\s*\Z", + re.IGNORECASE, +) + + +def confirms(text: str) -> bool: + """A typed yes to the proposal just made, instead of clicking Run.""" + return bool(_CONFIRMS.match(text)) + + @dataclass(frozen=True) class Overrepresentation: """What the reader is told, and whether there was anything to continue.""" @@ -294,3 +425,9 @@ def describe_overrepresentation( "Reactome's Analysis Service didn't return a result. Please try again in " "a moment." ) + + +EXPIRED = ( + "That list is no longer waiting to be analysed. Send it again and I'll " + "offer to run it." +) diff --git a/src/gsa/chat.py b/src/gsa/chat.py index ecfe3ab..dc7fbd2 100644 --- a/src/gsa/chat.py +++ b/src/gsa/chat.py @@ -207,10 +207,18 @@ def asks_to_run_gsa(text: str) -> bool: #: The gene-list line promises what `analysis.gene_list` does. It first #: pointed at the website, because the chat did not run that analysis yet. -HOW_TO_RUN_GSA = """Yes β€” you can run a gene set analysis right here in the chat, using ReactomeGSA. +#: Uploading a matrix, and nothing else. What "No, answer my question" gets +#: when the declined message also asked about GSA: the gene-list line +#: would offer again the analysis the reader has just turned down. +HOW_TO_RUN_GSA_WITH_A_MATRIX = """Yes β€” you can run a gene set analysis right here in the chat, using ReactomeGSA. 1. **Attach your expression matrix** with the πŸ“Ž button below: a `.tsv` or `.csv` file with genes (or proteins) as rows, samples as columns, and a first row naming the samples. Up to 20 MB. 2. **Tell me which group each sample is in** when I ask β€” for example `control, control, treated, treated`. -3. I'll run the analysis and give you the most significant pathways, a link to view the result in Reactome's Pathway Browser, and the full results table to download. It usually takes a few minutes. +3. I'll run the analysis and give you the most significant pathways, a link to view the result in Reactome's Pathway Browser, and the full results table to download. It usually takes a few minutes.""" + +HOW_TO_RUN_GSA = ( + HOW_TO_RUN_GSA_WITH_A_MATRIX + + """ If you only have a **list of genes** rather than measurements for each sample, that's an over-representation analysis instead, and I can run it here too: ask me to analyse them and include the genes in your message β€” for example *run a pathway analysis on TP53, ERBB2, RUNX2*.""" +) diff --git a/tests/analysis/gene_list_phrases.py b/tests/analysis/gene_list_phrases.py index 4229f6f..95e0da7 100644 --- a/tests/analysis/gene_list_phrases.py +++ b/tests/analysis/gene_list_phrases.py @@ -6,6 +6,13 @@ - HELD_OUT_*: written after that rewrite and never tuned against. They are the honest measure: 15/15 and 0/20 when added. +- SECOND_REVIEW_*: from a second review, of the version that proposed + rather than ran. It fired on 12 of 40 questions and read the wrong list + or none for 3 of 25; the parser was rewritten again. +- HELD_OUT_3_*: written after that, never tuned against: 11/12 requests + exact, 1/15 questions fired. The two failures are KNOWN_LIMITS, pinned + as they are rather than tuned away, so the rate stays honest. + A positive is exact: the identifiers read, in order. A request that reads the wrong list is a failure even if it fires. """ @@ -209,3 +216,231 @@ "Which of these involve TP53?", "Can you find pathways where both INS and INSR are present?", ] + + +SECOND_REVIEW_REQUESTS: list[tuple[str, list[str]]] = [ + ("run ORA on\nTP53\tMDM2\tCDKN1A\tBAX", ["TP53", "MDM2", "CDKN1A", "BAX"]), + ( + "Enrichment please:\nGene\nTP53\nMDM2\nCDKN1A\nBAX", + ["TP53", "MDM2", "CDKN1A", "BAX"], + ), + ("analyse these genes:\n1. TP53\n2. MDM2\n3. CDKN1A", ["TP53", "MDM2", "CDKN1A"]), + ("please run an enrichment on:\n- EGFR\n- KRAS\n- BRAF", ["EGFR", "KRAS", "BRAF"]), + ("perform pathway analysis on:\n* EGFR\n* KRAS\n* BRAF", ["EGFR", "KRAS", "BRAF"]), + ("run ORA on TP53, P38398, ENSG00000135679", ["TP53", "P38398", "ENSG00000135679"]), + ( + "run an enrichment on TP53, MDM2, CDKN1A. These came from my knockdown screen.", + ["TP53", "MDM2", "CDKN1A"], + ), + ( + "Run a pathway analysis on these: TP53, MDM2, BAX, thanks!", + ["TP53", "MDM2", "BAX"], + ), + ( + "do an ORA with the following genes\nSTAT1\nSTAT2\nIRF9\n\nThey are interferon genes.", + ["STAT1", "STAT2", "IRF9"], + ), + ("run enrichment on MYC/MAX/MXD1", ["MYC", "MAX", "MXD1"]), + ("Run ORA on: TP53 | MDM2 | CDKN1A", ["TP53", "MDM2", "CDKN1A"]), + ( + "Please do an enrichment analysis on my gene list (TP53, MDM2, CDKN1A)", + ["TP53", "MDM2", "CDKN1A"], + ), + ('run ora on "TP53", "MDM2", "CDKN1A"', ["TP53", "MDM2", "CDKN1A"]), + ( + "Run an enrichment for HLA-A, HLA-B, B2M, TAP1", + ["HLA-A", "HLA-B", "B2M", "TAP1"], + ), + ( + "Can you run an ORA on these mouse genes: Trp53, Mdm2, Cdkn1a, Bax", + ["Trp53", "Mdm2", "Cdkn1a", "Bax"], + ), + ( + "perform enrichment on CD8A, GZMB, PRF1, IFNG, and NKG7", + ["CD8A", "GZMB", "PRF1", "IFNG", "NKG7"], + ), + ( + "Run a Reactome analysis on the list below\n\nIL6\nIL1B\nTNF\nCXCL8", + ["IL6", "IL1B", "TNF", "CXCL8"], + ), + ("find enriched pathways for: sox9, runx2, sp7", ["sox9", "runx2", "sp7"]), + ( + "run GSEA on TP53, MDM2, CDKN1A, BAX, BBC3, PMAIP1, FAS, TNFRSF10B, GADD45A, SESN1, RRM2B, ZMAT3", + [ + "TP53", + "MDM2", + "CDKN1A", + "BAX", + "BBC3", + "PMAIP1", + "FAS", + "TNFRSF10B", + "GADD45A", + "SESN1", + "RRM2B", + "ZMAT3", + ], + ), + ("run an ORA on P04637-2, Q00987, O15350", ["P04637-2", "Q00987", "O15350"]), + ( + "Please run enrichment on ATF4 DDIT3 XBP1 ERN1 EIF2AK3", + ["ATF4", "DDIT3", "XBP1", "ERN1", "EIF2AK3"], + ), + ( + "analyse: ENSG00000141510.18, ENSG00000135679.25", + ["ENSG00000141510", "ENSG00000135679"], + ), + ( + "run ORA with genes GATA1, TAL1, KLF1, LMO2 from my erythroid dataset", + ["GATA1", "TAL1", "KLF1", "LMO2"], + ), + ( + "Do an over-representation analysis for PSEN1, APP, APOE, MAPT, TREM2, CLU", + ["PSEN1", "APP", "APOE", "MAPT", "TREM2", "CLU"], + ), +] + +SECOND_REVIEW_QUESTIONS: list[str] = [ + "Can EGFR and ERBB2 form heterodimers in Reactome?", + "Are there any pathways where both TP53 and MYC act?", + "Please list the reactions that SMAD3 and SMAD4 participate in", + "I'd like the Reactome pathways for CDK4 and CDK6", + "Which Reactome pathway has the most overlap with BRCA1, BRCA2 and PALB2?", + "Find me papers about KRAS and NRAS in colorectal cancer", + "Does Reactome have an analysis of IL6 versus IL10 signalling?", + "Show me where PIK3CA, AKT1 and MTOR sit in the PI3K pathway", + "Map out the interactions between NOTCH1 and HES1 for me", + "Run a search for pathways containing JAK1 and JAK2", + "Which pathways are shared by STAT1 and STAT3?", + "In my enrichment analysis, CXCL8 and CCL2 were top hits. Anything to worry about?", + "Find the reactions where MDM2 ubiquitinates TP53", + "Can you find the complexes that contain CDK1 and CCNB1?", + "I performed an ORA with 200 genes, of which TP53 and ATM were most significant. Next steps?", + "Submit a question: are ESR1 and FOXA1 co-regulated?", + "Please run a check on whether BAX and BAK1 are in apoptosis", + "Map the protein P04637 and P38398 to their Reactome names", + "Find pathways related to insulin, e.g. INS, INSR", + "Which pathways are regulated by miRNAs targeting PTEN and TP53?", + "Is the GSEA leading edge with ACTB and GAPDH normal?", + "Execute a query for interactors of VEGFA and KDR", + "Can I analyse a gene list of 500 genes including TP53 and EGFR?", + "Can you find what pathways CD4 and CD8A are enriched for in T cells, per the literature?", + "Which pathways are my DE genes in? I have not decided yet, maybe TP53 and MYC.", + "I want to run ORA later; first, what are HIF1A and EPAS1?", + "Find enrichment papers on BRCA1 and BRCA2 please", + "Run me a summary: TP53, MDM2", + "Do an analysis of the TP53-MDM2 feedback loop", + "Please analyse the role of TNF and IL1B in inflammation", + "Analyse the relationship between APC and CTNNB1", + "Which pathways do SOX2 and NANOG regulate?", + "Find the enrichment map for my results; genes include FOS and JUN", + "Can you perform a literature search on GSEA results with MYC and E2F1?", + "Carry out a comparison between AKT1 and AKT2 please", + "Which pathways involve TGFB1 and SMAD7 negatively?", + "Map EGFR mutations L858R and T790M to pathways", + "Run through the analysis steps for MAPK1 and MAPK3", + "Find the pathways on chromosome 17 with TP53 and BRCA1", + "Is ORA appropriate for TP53, MDM2 and CDKN1A or should I use GSEA?", +] + +HELD_OUT_3_REQUESTS: list[tuple[str, list[str]]] = [ + ( + "Hi! Could you run a pathway enrichment on NOTCH1, NOTCH2, JAG1, DLL4, HES1?", + ["NOTCH1", "NOTCH2", "JAG1", "DLL4", "HES1"], + ), + ( + "run ORA:\nABCA1\nABCG1\nAPOA1\nLDLR\nPCSK9", + ["ABCA1", "ABCG1", "APOA1", "LDLR", "PCSK9"], + ), + ( + "please analyse this list: Myod1, Myog, Myf5, Des", + ["Myod1", "Myog", "Myf5", "Des"], + ), + ( + "Can you do an enrichment analysis on CXCR4, CXCL12, ACKR3?", + ["CXCR4", "CXCL12", "ACKR3"], + ), + ("run pathway analysis on P01308, P06213, P35568", ["P01308", "P06213", "P35568"]), + ( + "Perform ORA on BCL2, BCL2L1, MCL1, BAX, BAK1, BID. Thanks in advance", + ["BCL2", "BCL2L1", "MCL1", "BAX", "BAK1", "BID"], + ), + ("find enriched pathways for\nhk1\nhk2\ngck\npfkl", ["hk1", "hk2", "gck", "pfkl"]), + ( + "run an enrichment on SREBF1; SREBF2; INSIG1; SCAP", + ["SREBF1", "SREBF2", "INSIG1", "SCAP"], + ), + ( + "Could we run a GSA with TNFRSF1A, TRADD, RIPK1 and TRAF2?", + ["TNFRSF1A", "TRADD", "RIPK1", "TRAF2"], + ), + ( + "analyse the following:\nCOL1A1\nCOL1A2\nCOL3A1\nFN1\n\nThese are fibrosis markers", + ["COL1A1", "COL1A2", "COL3A1", "FN1"], + ), + ( + "do a pathway analysis with ATG5, ATG7, BECN1, MAP1LC3B, SQSTM1", + ["ATG5", "ATG7", "BECN1", "MAP1LC3B", "SQSTM1"], + ), +] + +HELD_OUT_3_QUESTIONS: list[str] = [ + "Could you explain whether SOX9 and RUNX2 act together?", + "Which Reactome pathway includes both ATG5 and ATG7?", + "What happens to BCL2 and BAX during apoptosis?", + "Can you run a quick check: is TP53 a tumour suppressor?", + "Do FOXP3 and IL2RA mark regulatory T cells?", + "Please find reactions catalysed by HK1 and HK2", + "Give me an overview of enrichment analysis for genes like CXCR4 and CXCL12", + "How do NOTCH1 and JAG1 signal?", + "Show me the pathway diagram containing LDLR and PCSK9", + "Which pathways would you expect to be enriched for COL1A1 and FN1?", + "Help me write a methods section for my ORA of TP53 and MDM2", + "What's the Reactome ID for SREBF1 and SREBF2?", + "Run a literature search on MYOD1 and MYOG", + "Can you describe what TNFRSF1A and TRADD do in NF-kB activation?", +] + +#: What a request followed by a trailing sentence must still read as. +TRAILING_BASE = "run an enrichment on TP53, MDM2, CDKN1A" +TRAILING: list[str] = [ + " from my screen", + " using default settings", + " thanks", + " cheers", + " asap", + " today", + " vs background", + " against the human genome", + "\n\nCheers, Anna", + "\nThanks", + " please and thank you", + " (human)", + " - human", + " if possible", + " when you can", + " at FDR 0.05", + " using Reactome", + " by FDR", + " sorted by pvalue", + " but exclude disease pathways", + " etc", + " genes", + "\n\nBest\nJohn", +] + +#: Current behaviour that is wrong, pinned so a change is noticed. +KNOWN_LIMITS: list[tuple[str, list[str] | None]] = [ + ( + "I'd like an over-representation analysis on these genes - FOXP3, IL2RA, CTLA4, IKZF2", + None, + ), + ( + "Is my list of TP53, MDM2 targets enriched for apoptosis? I have not run anything yet, just asking", + ["TP53", "MDM2"], + ), + ( + "gene\tlog2FC\tpadj\nTP53\t2.1\t0.001\nMDM2\t1.5\t0.01\nCDKN1A\t3.2\t0.0001\nplease run an enrichment analysis", + None, + ), +] diff --git a/tests/analysis/test_gene_list.py b/tests/analysis/test_gene_list.py index 3a76e18..2543cfa 100644 --- a/tests/analysis/test_gene_list.py +++ b/tests/analysis/test_gene_list.py @@ -5,6 +5,7 @@ """ import asyncio +import time from collections.abc import Callable import gene_list_phrases as phrases @@ -12,8 +13,10 @@ import pytest from analysis import client as analysis_client +from analysis.client import MAX_SUBMITTED_IDENTIFIERS from analysis.gene_list import ( MAX_PROPOSED_LISTED, + confirms, describe_overrepresentation, describe_proposal, gene_list_request, @@ -39,6 +42,23 @@ ), *phrases.REVIEW_REQUESTS, *phrases.HELD_OUT_REQUESTS, + *phrases.SECOND_REVIEW_REQUESTS, + *phrases.HELD_OUT_3_REQUESTS, + *( + (phrases.TRAILING_BASE + tail, ["TP53", "MDM2", "CDKN1A"]) + for tail in phrases.TRAILING + ), + ( + "Run an enrichment on up-regulated genes: TP53, MDM2; down-regulated: MYC, CCND1", + ["TP53", "MDM2", "MYC", "CCND1"], + ), + ("Run ORA on\r\nTP53\r\nMDM2", ["TP53", "MDM2"]), + # A list can open the message, in lower case. + ("egfr, kras, braf - run ORA on these", ["egfr", "kras", "braf"]), + # The verb is not the list's first member. + ("submit TLR4, MYD88 for pathway analysis", ["TLR4", "MYD88"]), + ("run ORA on TP53\u00a0MDM2\u00a0CDKN1A", ["TP53", "MDM2", "CDKN1A"]), + ("run ORA on TP53\uff0cMDM2\uff0cCDKN1A", ["TP53", "MDM2", "CDKN1A"]), ] NOT_REQUESTS = [ @@ -56,9 +76,70 @@ "", *phrases.REVIEW_QUESTIONS, *phrases.HELD_OUT_QUESTIONS, + *phrases.SECOND_REVIEW_QUESTIONS, + *phrases.HELD_OUT_3_QUESTIONS, + "please run GSA on my matrix, columns are sample1, sample2, sample3", + "Map EGFR mutations L858R and T790M to pathways", ] +@pytest.mark.parametrize(("text", "current"), phrases.KNOWN_LIMITS) +def test_known_limits_are_as_recorded(text: str, current: list[str] | None) -> None: + # Wrong, and pinned: if one of these changes, update the record and the + # measured rate in gene_list_phrases.py. + assert gene_list_request(text) == current + + +@pytest.mark.parametrize( + "text", + [ + "run ORA on:\n" + "\n".join(f"GENE{i}" for i in range(8000)), + "run ORA on TP53, MDM2" + "\n" * 50_000 + "x", + "run ORA on TP53, MDM2" + "?" * 50_000 + "x", + "run ORA on: TP53, MDM2" + " " * 50_000 + "!MYC", + "run ORA on " + "of " * 15_000 + "TP53, MDM2", + "do " + "word " * 11_000 + "analysis on TP53, MDM2", + # Over the cap: refused unread, however list-like. + "run ORA on: " + "TP53, " * 500_000, + ], + ids=[ + "column", + "newlines", + "question-marks", + "spaces", + "openers", + "request", + "huge", + ], +) +def test_reading_a_message_is_fast_whatever_it_holds(text: str) -> None: + # It runs on the event loop every session shares. The first rewrite took + # 38 s on a 4,000-line column and 24 s on 40,000 newlines. + started = time.perf_counter() + gene_list_request(text) + assert time.perf_counter() - started < 0.5 + + +@pytest.mark.parametrize( + "text", ["yes", "Yes please", "ok", "run it", "Go ahead!", "sure."] +) +def test_a_typed_yes_confirms(text: str) -> None: + assert confirms(text) + + +@pytest.mark.parametrize( + "text", ["yes but first explain ORA", "no", "what is TP53?", "", "yesterday"] +) +def test_anything_else_does_not(text: str) -> None: + assert not confirms(text) + + +def test_the_proposal_says_when_the_list_will_be_cut() -> None: + long = describe_proposal([f"G{i}" for i in range(MAX_SUBMITTED_IDENTIFIERS + 1)]) + assert f"Only the first {MAX_SUBMITTED_IDENTIFIERS:,} will be submitted" in long + assert "will be submitted" not in describe_proposal(["TP53", "MDM2"]) + + @pytest.mark.parametrize(("text", "expected"), REQUESTS) def test_a_request_with_genes_is_recognised(text: str, expected: list[str]) -> None: assert gene_list_request(text) == expected @@ -71,8 +152,8 @@ def test_other_messages_are_left_to_the_model(text: str) -> None: def test_the_sets_are_the_size_they_claim() -> None: # Sized sets are the point: a pass at n=2 says nothing about a rate. - assert len(REQUESTS) >= 40 - assert len(NOT_REQUESTS) >= 70 + assert len(REQUESTS) >= 100 + assert len(NOT_REQUESTS) >= 120 def test_the_proposal_lists_what_will_be_submitted() -> None: diff --git a/tests/gsa/test_gsa_chat.py b/tests/gsa/test_gsa_chat.py index 52e1f11..d31bb56 100644 --- a/tests/gsa/test_gsa_chat.py +++ b/tests/gsa/test_gsa_chat.py @@ -280,3 +280,12 @@ def test_the_gene_list_example_it_gives_is_one_the_chat_runs() -> None: example = re.search(r"\*(run a pathway analysis on [^*]+)\*", chat.HOW_TO_RUN_GSA) assert example is not None assert gene_list_request(example.group(1)) == ["TP53", "ERBB2", "RUNX2"] + + +def test_the_matrix_only_reply_offers_no_gene_list() -> None: + # Sent after the reader declined a gene-list analysis; it must not offer + # that analysis again. + assert chat.HOW_TO_RUN_GSA.startswith(chat.HOW_TO_RUN_GSA_WITH_A_MATRIX) + assert "Attach" in chat.HOW_TO_RUN_GSA_WITH_A_MATRIX + assert "list of genes" not in chat.HOW_TO_RUN_GSA_WITH_A_MATRIX + assert "list of genes" in chat.HOW_TO_RUN_GSA From 23b1efaeec6d082aecb45f71cd33c004127bd0a0 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 25 Sep 2026 21:11:24 +0000 Subject: [PATCH 4/4] Keep offers out of the saved session, and run button work as a task Round three of adversarial review: - Resuming a thread and typing yes raised AttributeError: offers lived in user_session, which Chainlit saves as JSON, and cl.Action became null. Offers are now plain data in a bounded in-process store. - Button callbacks ran with no task: no Stop, sending not blocked, errors swallowed, and the work held the action HTTP request open. They now run as a stoppable task that reports failure, as a message does. - No after a restart said nothing; it now says the offer lapsed. Evicted offers lose their buttons. - The typed-yes state is read and cleared before any early return. - A lower-case word after a list in capitals is the sentence going on ("TP53, MDM2, then show me..."); a lone hyphen is a separator; "in" opens a list; two genes in a question are its subject, not a list. - The column timing test now reaches the parser (it was over the cap). Measured on a fourth set written after these fixes and never tuned against: 9/10 requests exact, 0/12 questions offered an analysis. Co-Authored-By: Claude Opus 5.5 --- bin/chat-chainlit.py | 105 +++++++++++------ src/analysis/gene_list.py | 36 ++++-- src/analysis/proposals.py | 89 +++++++++++++++ tests/analysis/gene_list_phrases.py | 170 +++++++++++++++++++++++++++- tests/analysis/test_gene_list.py | 19 +++- tests/analysis/test_proposals.py | 86 ++++++++++++++ 6 files changed, 458 insertions(+), 47 deletions(-) create mode 100644 src/analysis/proposals.py create mode 100644 tests/analysis/test_proposals.py diff --git a/bin/chat-chainlit.py b/bin/chat-chainlit.py index 63d0dc1..1c07b81 100644 --- a/bin/chat-chainlit.py +++ b/bin/chat-chainlit.py @@ -2,11 +2,13 @@ # chainlit ships no annotations for cl.user_session.get/set, cl.Message.send # or get_data_layer. Not fixable here; it needs stubs upstream. The file is # named with a hyphen, so it cannot be listed in [[tool.mypy.overrides]]. +import asyncio import os -import uuid +from collections.abc import Awaitable, Callable from pathlib import Path import chainlit as cl +from chainlit.context import context from chainlit.data.base import BaseDataLayer from chainlit.data.sql_alchemy import SQLAlchemyDataLayer from chainlit.oauth_providers import providers @@ -26,6 +28,7 @@ pathway_browser_url, submit_identifiers, ) +from analysis.proposals import Proposal, proposals from gsa.chainlit_flow import ( Attachment, matrix_attachment, @@ -275,8 +278,14 @@ def current_thread_id() -> str: return str(cl.user_session.get("thread_id") or cl.user_session.get("id")) -#: Proposals kept per session, so an older proposal's buttons still work. -MAX_PENDING_PROPOSALS = 5 +def session_id() -> str: + return str(cl.user_session.get("id")) + + +async def remove_buttons(proposal: Proposal) -> None: + # Rebuilt from their ids: the frontend removes an action by id. + for name, action_id in proposal.actions: + await cl.Action(name=name, payload={}, id=action_id).remove() async def propose_gene_list(message: cl.Message, identifiers: list[str]) -> None: @@ -286,65 +295,92 @@ async def propose_gene_list(message: cl.Message, identifiers: list[str]) -> None clicks, and drops the question if they never do. Here they can click Run, click No, type "yes", or simply type on. """ - proposal_id = uuid.uuid4().hex + proposal_id = proposals.new_id() run = cl.Action(name="gene_list_run", payload={"id": proposal_id}, label="Run it") decline = cl.Action( name="gene_list_no", payload={"id": proposal_id}, label="No, answer my question", ) - pending: dict[str, dict] = cl.user_session.get("gene_list_pending") or {} - pending[proposal_id] = { - "text": message.content, - "message_id": message.id, - "identifiers": identifiers, - "actions": [run, decline], - } - while len(pending) > MAX_PENDING_PROPOSALS: - pending.pop(next(iter(pending))) - cl.user_session.set("gene_list_pending", pending) - # A typed "yes" means the proposal just made, and only that one. - cl.user_session.set("gene_list_latest", proposal_id) + evicted = proposals.put( + session_id(), + proposal_id, + Proposal( + text=message.content, + message_id=message.id, + identifiers=tuple(identifiers), + actions=((run.name, run.id), (decline.name, decline.id)), + ), + ) + for old in evicted: + await remove_buttons(old) await cl.Message( content=gene_list.describe_proposal(identifiers), actions=[run, decline] ).send() -async def take_proposal(proposal_id: str | None) -> dict | None: - """Claim a proposal once, and take its buttons away.""" - pending: dict[str, dict] = cl.user_session.get("gene_list_pending") or {} - proposal = pending.pop(proposal_id, None) if proposal_id else None - cl.user_session.set("gene_list_pending", pending) - if cl.user_session.get("gene_list_latest") == proposal_id: - cl.user_session.set("gene_list_latest", None) +async def take_proposal(proposal_id: str | None) -> Proposal | None: + """Claim an offer once, and take its buttons away.""" + proposal = proposals.take(session_id(), proposal_id) if proposal is not None: - for action in proposal["actions"]: - await action.remove() + await remove_buttons(proposal) return proposal +def run_as_task(work: Callable[[], Awaitable[None]]) -> None: + """Run a button's work the way Chainlit runs a message. + + Action callbacks run with no task: the message box stays open, there is + no Stop, an exception is logged and the reader told nothing, and the + work holds the /project/action HTTP request open. So: mark a task, make + it stoppable, report failure, and return from the request at once. + """ + + async def body() -> None: + await context.emitter.task_start() + try: + await work() + except asyncio.CancelledError: + pass + except Exception: + logger.exception("gene list action failed") + await cl.ErrorMessage(content=gene_list.FAILED_TO_ANSWER).send() + finally: + await context.emitter.task_end() + + context.session.current_task = asyncio.create_task(body()) + + @cl.action_callback("gene_list_run") async def on_gene_list_run(action: cl.Action) -> None: proposal = await take_proposal(action.payload.get("id")) if proposal is None: await cl.Message(content=gene_list.EXPIRED).send() return - await run_gene_list_analysis(proposal["text"], proposal["identifiers"]) + run_as_task( + lambda: run_gene_list_analysis(proposal.text, list(proposal.identifiers)) + ) @cl.action_callback("gene_list_no") async def on_gene_list_no(action: cl.Action) -> None: proposal = await take_proposal(action.payload.get("id")) if proposal is None: + # After a restart the offer is gone, and so is the question's text. + await cl.Message(content=gene_list.EXPIRED_DECLINED).send() return + run_as_task(lambda: decline_gene_list(proposal)) + + +async def decline_gene_list(proposal: Proposal) -> None: # Not the full GSA how-to: for "can we do a gsa analysis... TP53, ERBB2" # it ends by offering to analyse the list just declined. Not the model # either: grounded in the website's user guide, it says GSA cannot be # done in this chat -- the bug the how-to exists to fix. Measured both. - if asks_to_run_gsa(proposal["text"]): + if asks_to_run_gsa(proposal.text): await cl.Message(content=HOW_TO_RUN_GSA_WITH_A_MATRIX).send() return - await answer_with_model(proposal["text"], proposal["message_id"]) + await answer_with_model(proposal.text, proposal.message_id) async def run_gene_list_analysis(text: str, identifiers: list[str]) -> None: @@ -462,6 +498,10 @@ async def answer_with_model(content: str, message_id: str) -> None: @cl.on_message async def main(message: cl.Message) -> None: + # First, before any early return: a "yes" means the offer just made, so + # any other message -- rate limited, an attachment -- ends that meaning. + latest = proposals.take_latest(session_id()) + if await message_rate_limited(config): return @@ -478,16 +518,15 @@ async def main(message: cl.Message) -> None: await run_gsa_analysis(attachment) return - # "yes" to the proposal just made is the same as clicking Run. Only the - # message straight after it: a "yes" later answers something else. - latest = cl.user_session.get("gene_list_latest") - cl.user_session.set("gene_list_latest", None) + # "yes" to the offer just made is the same as clicking Run. if latest and gene_list.confirms(message.content or ""): proposal = await take_proposal(latest) if proposal is not None: - await run_gene_list_analysis(proposal["text"], proposal["identifiers"]) + await run_gene_list_analysis(proposal.text, list(proposal.identifiers)) return + # "yes" to the proposal just made is the same as clicking Run. Only the + # message straight after it: a "yes" later answers something else. # A gene list is not a matrix: offer what Reactome runs on a list, # over-representation, rather than explaining how to upload a matrix. # Before the GSA check, because the request that prompted this said "gsa". diff --git a/src/analysis/gene_list.py b/src/analysis/gene_list.py index 47769a0..8349bbd 100644 --- a/src/analysis/gene_list.py +++ b/src/analysis/gene_list.py @@ -106,7 +106,7 @@ #: Words a list follows: "for", "on", "genes", and the request verbs #: themselves ("analyze TP53, MDM2"). _OPENER_WORDS = frozenset( - """FOR ON OF WITH GENES PROTEINS LIST THESE FOLLOWING RUN PERFORM SUBMIT + """FOR ON OF IN WITH GENES PROTEINS LIST THESE FOLLOWING RUN PERFORM SUBMIT ANALYSE ANALYZE MAP FIND""".split() ) #: Joining words, read as part of the gap between two tokens. @@ -185,7 +185,7 @@ class _Run: ended_by_and: bool = False -def _accepted(run: _Run, *, shouting: bool, pair_ok: bool) -> list[str]: +def _accepted(run: _Run, *, shouting: bool, pair_ok: bool, question: bool) -> list[str]: tokens = run.tokens # Space-separated plain words are prose unless a colon, question mark or # newline announced a list: "on TP53 MDM2 using default settings" reads @@ -197,10 +197,21 @@ def _accepted(run: _Run, *, shouting: bool, pair_ok: bool) -> list[str]: ): tokens = tokens[:index] break + # A list typed in lower case is read in lower case. In any other list a + # lower-case word is the sentence carrying on: "TP53, MDM2, then show + # me the top hits", "..., cheers". + if any(token != token.lower() for token, _ in tokens): + for index, (token, strength) in enumerate(tokens): + if strength == "weak" and token == token.lower(): + tokens = tokens[:index] + break if len(tokens) < MIN_IDENTIFIERS: return [] # "X and Y" is how prose names two genes; a list says it with commas. - if len(tokens) == 2 and run.kind == "and" and not pair_ok: + # And two genes in a question are the question's subject -- "My + # enrichment for HIF1A, VEGFA came back empty. What went wrong?" -- more + # often than a list to run: a third review's four misfires were all this. + if len(tokens) == 2 and (run.kind == "and" or question) and not pair_ok: return [] return [token for token, _ in tokens] @@ -217,7 +228,8 @@ def identifiers_in(text: str, *, shouting: bool = False) -> list[str]: """ text = text.replace("\r\n", "\n").translate(_NORMALISE) text = _ENSEMBL_VERSION.sub(r"\1", _MARKERS.sub("", text)) - pair_ok = "?" not in text and bool(_REQUEST_START.match(text)) + question = "?" in text + pair_ok = not question and bool(_REQUEST_START.match(text)) found: list[str] = [] run: _Run | None = None previous_end = 0 @@ -225,12 +237,15 @@ def identifiers_in(text: str, *, shouting: bool = False) -> list[str]: def close() -> None: if run is not None: - found.extend(_accepted(run, shouting=shouting, pair_ok=pair_ok)) + found.extend( + _accepted(run, shouting=shouting, pair_ok=pair_ok, question=question) + ) for match in _TOKEN.finditer(text): token = match.group().strip("-") - if token.lower() in _JOINERS: - continue # stays in the gap: "TP53, ERBB2 and RUNX2" + if not token or token.lower() in _JOINERS: + # Stays in the gap: "TP53, ERBB2 and RUNX2", "my genes - CTNNB1". + continue gap = text[previous_end : match.start()] previous_end = match.end() if _BLANK_LINE.search(gap): @@ -431,3 +446,10 @@ def describe_overrepresentation( "That list is no longer waiting to be analysed. Send it again and I'll " "offer to run it." ) + +EXPIRED_DECLINED = ( + "That offer has lapsed, so I no longer have the question that went with " + "it. Please ask it again." +) + +FAILED_TO_ANSWER = "Something went wrong answering that. Please try again." diff --git a/src/analysis/proposals.py b/src/analysis/proposals.py new file mode 100644 index 0000000..1192fb6 --- /dev/null +++ b/src/analysis/proposals.py @@ -0,0 +1,89 @@ +"""Gene-list analyses the chat has offered and the reader has not yet answered. + +In process memory, not in Chainlit's `user_session`. The session is saved +into the thread's metadata as JSON: `cl.Action` objects became `null`, so +resuming a thread and typing "yes" raised on `None.remove()`, and five +60K-character messages would have been written into every thread's metadata +besides. Nothing here needs to outlive the process -- after a restart the +buttons say the offer has lapsed, and the reader sends the list again. + +Plain data only, and bounded twice: offers per session, and sessions. +""" + +import uuid +from collections import OrderedDict +from dataclasses import dataclass, field + +MAX_PER_SESSION = 5 +MAX_SESSIONS = 2000 + + +@dataclass(frozen=True) +class Proposal: + #: The reader's message, for the model if they decline. + text: str + message_id: str + identifiers: tuple[str, ...] + #: `(name, id)` of each button, so they can be taken away once answered. + actions: tuple[tuple[str, str], ...] + + +@dataclass +class _Session: + offers: OrderedDict[str, Proposal] = field(default_factory=OrderedDict) + #: The offer a typed "yes" means: the one just made, and only that. + latest: str | None = None + + +@dataclass +class ProposalStore: + max_per_session: int = MAX_PER_SESSION + max_sessions: int = MAX_SESSIONS + _sessions: OrderedDict[str, _Session] = field(default_factory=OrderedDict) + + def _session(self, session_id: str) -> _Session: + found = self._sessions.get(session_id) + if found is None: + found = self._sessions[session_id] = _Session() + while len(self._sessions) > self.max_sessions: + self._sessions.popitem(last=False) + self._sessions.move_to_end(session_id) + return found + + @staticmethod + def new_id() -> str: + return uuid.uuid4().hex + + def put( + self, session_id: str, proposal_id: str, proposal: Proposal + ) -> list[Proposal]: + """Keep an offer; returns any pushed out, whose buttons should go.""" + session = self._session(session_id) + session.offers[proposal_id] = proposal + session.latest = proposal_id + evicted: list[Proposal] = [] + while len(session.offers) > self.max_per_session: + evicted.append(session.offers.popitem(last=False)[1]) + return evicted + + def take(self, session_id: str, proposal_id: str | None) -> Proposal | None: + """Claim an offer, once. No await between the lookup and the pop, so + a double click or a click plus a typed "yes" cannot run it twice.""" + session = self._sessions.get(session_id) + if session is None or not proposal_id: + return None + if session.latest == proposal_id: + session.latest = None + return session.offers.pop(proposal_id, None) + + def take_latest(self, session_id: str) -> str | None: + """The offer a "yes" in this message would mean -- and forget it, so + only the message straight after an offer can confirm it.""" + session = self._sessions.get(session_id) + if session is None: + return None + latest, session.latest = session.latest, None + return latest + + +proposals = ProposalStore() diff --git a/tests/analysis/gene_list_phrases.py b/tests/analysis/gene_list_phrases.py index 95e0da7..6f0d153 100644 --- a/tests/analysis/gene_list_phrases.py +++ b/tests/analysis/gene_list_phrases.py @@ -10,8 +10,16 @@ rather than ran. It fired on 12 of 40 questions and read the wrong list or none for 3 of 25; the parser was rewritten again. - HELD_OUT_3_*: written after that, never tuned against: 11/12 requests - exact, 1/15 questions fired. The two failures are KNOWN_LIMITS, pinned - as they are rather than tuned away, so the rate stays honest. + exact, 1/15 questions fired. The miss and a pasted table (which + should be attached as a file) are KNOWN_LIMITS, pinned as they are. + The misfire was later fixed, by the rule for round three's misfires. +- A third review's fresh set, on the version after that: 18/20 requests + exact, 4/20 questions offered an analysis. The offer costs one click; + the rate is the honest one to quote. +- THIRD_REVIEW_*: that review's fresh set, then tuned against (4 + question misfires, all two genes in a question, fixed by one rule). +- HELD_OUT_4_*: written after round three's fixes, never tuned against: + 9/10 requests exact, 0/12 questions offered. The miss is a KNOWN_LIMIT. A positive is exact: the identifiers read, in order. A request that reads the wrong list is a failure even if it fires. @@ -385,6 +393,8 @@ ] HELD_OUT_3_QUESTIONS: list[str] = [ + # Was a known limit; fixed by the two-genes-in-a-question rule. + "Is my list of TP53, MDM2 targets enriched for apoptosis? I have not run anything yet, just asking", "Could you explain whether SOX9 and RUNX2 act together?", "Which Reactome pathway includes both ATG5 and ATG7?", "What happens to BCL2 and BAX during apoptosis?", @@ -429,18 +439,166 @@ "\n\nBest\nJohn", ] +THIRD_REVIEW_REQUESTS: list[tuple[str, list[str]]] = [ + ( + "Could you run an over-representation analysis for SOX2, POU5F1, NANOG, KLF4 and LIN28A?", + ["SOX2", "POU5F1", "NANOG", "KLF4", "LIN28A"], + ), + ( + "run enrichment on these DE genes:\nIL6\nCXCL8\nCCL2\nTNF\nIL1B\nPTGS2", + ["IL6", "CXCL8", "CCL2", "TNF", "IL1B", "PTGS2"], + ), + ("Perform ORA with P04637, Q00987, P38936", ["P04637", "Q00987", "P38936"]), + ( + "please do a reactome analysis of ENSG00000141510, ENSG00000135679, ENSG00000124762", + ["ENSG00000141510", "ENSG00000135679", "ENSG00000124762"], + ), + ( + "map BRCA1 BRCA2 PALB2 RAD51C RAD51D to pathways", + ["BRCA1", "BRCA2", "PALB2", "RAD51C", "RAD51D"], + ), + ( + "Find the enriched pathways for my hits: Atg5, Atg7, Becn1, Map1lc3b, Sqstm1", + ["Atg5", "Atg7", "Becn1", "Map1lc3b", "Sqstm1"], + ), + ( + "hi! can you run a pathway enrichment on CD3E, CD4, CD8A, GZMB, PRF1, IFNG thanks", + ["CD3E", "CD4", "CD8A", "GZMB", "PRF1", "IFNG"], + ), + ( + "Run ORA:\nHIF1A, VEGFA, EPAS1, LDHA, PDK1, SLC2A1", + ["HIF1A", "VEGFA", "EPAS1", "LDHA", "PDK1", "SLC2A1"], + ), + ( + "I'd like you to analyse NOTCH1, HES1, HEY1, JAG1, DLL4 for pathway enrichment", + ["NOTCH1", "HES1", "HEY1", "JAG1", "DLL4"], + ), + ( + "Submit this list to Reactome analysis: MLH1; MSH2; MSH6; PMS2", + ["MLH1", "MSH2", "MSH6", "PMS2"], + ), + ( + "do an enrichment analysis on the following genes: ATM, ATR, CHEK1, CHEK2, WEE1", + ["ATM", "ATR", "CHEK1", "CHEK2", "WEE1"], + ), + ( + "run gsa with KEAP1, NFE2L2, NQO1, HMOX1, GCLM", + ["KEAP1", "NFE2L2", "NQO1", "HMOX1", "GCLM"], + ), + ( + "Which pathways are enriched in STAT1 STAT2 IRF9 ISG15 MX1 OAS1?", + ["STAT1", "STAT2", "IRF9", "ISG15", "MX1", "OAS1"], + ), + ( + "Run enrichment for:\n1. PINK1\n2. PRKN\n3. LRRK2\n4. SNCA\n5. PARK7", + ["PINK1", "PRKN", "LRRK2", "SNCA", "PARK7"], + ), + ( + "analyze my genes - CTNNB1, APC, AXIN2, LGR5, TCF7L2 - using reactome", + ["CTNNB1", "APC", "AXIN2", "LGR5", "TCF7L2"], + ), + ( + "please perform over representation analysis: Tp53, Mdm2, Cdkn1a, Bax, Pmaip1", + ["Tp53", "Mdm2", "Cdkn1a", "Bax", "Pmaip1"], + ), + ( + "run an ORA on SMAD2/SMAD3/SMAD4/TGFBR1/TGFBR2", + ["SMAD2", "SMAD3", "SMAD4", "TGFBR1", "TGFBR2"], + ), + ( + "Can you do pathway analysis on my list? MYOD1, MYOG, MEF2C, DES, ACTA1", + ["MYOD1", "MYOG", "MEF2C", "DES", "ACTA1"], + ), + ( + "execute an enrichment on EZH2 SUZ12 EED RBBP4", + ["EZH2", "SUZ12", "EED", "RBBP4"], + ), + ( + "Analyse for pathway enrichment:\nGAPDH\nPGK1\nENO1\nPKM\nALDOA\n\nThanks,\nMaria", + ["GAPDH", "PGK1", "ENO1", "PKM", "ALDOA"], + ), +] + +THIRD_REVIEW_QUESTIONS: list[str] = [ + "Is KRAS or NRAS more frequently mutated in colorectal cancer pathways?", + "Can you find pathways where both PTEN and PIK3CA act?", + "What happens downstream of EGFR and ERBB2 activation?", + "Which pathways does TP53 participate in, and does MDM2 share any?", + "Run me through the steps of mismatch repair involving MLH1 and MSH2", + "Has anyone performed an enrichment analysis with BRCA1 and BRCA2 knockouts in Reactome?", + "I ran ORA on my list and got Signaling by NOTCH at the top with NOTCH1, JAG1 - is that plausible?", + "Do SOX2, POU5F1 appear together in any Reactome pathway?", + "After I run the analysis on IL6, TNF, should I use the projection to human?", + "Which pathways are shown when I analyze CD19, MS4A1 in the Pathway Browser - where do I click?", + "What's the Reactome ID for the pathway containing ATM, CHEK2?", + "My enrichment for HIF1A, VEGFA came back empty. What went wrong?", + "Map of the interactions between KEAP1 and NFE2L2 please", + "Could analysis of STAT3, JAK2 phosphorylation be done in Reactome?", + "Is GAPDH a good housekeeping gene for ORA background, along with ACTB?", + "In which pathways would BCL2, BAX, BAK1 be found?", + "Please find me literature-backed pathways for FOXO1, FOXO3", + "Should I run an ORA with SMAD4, TGFBR2 or wait until I have more genes?", + "Why doesn't my ORA list CDK4, CDK6 in cell cycle?", + "Run the numbers for me: are MYC, MAX, MXD1 all in the same Reactome pathway?", +] + +HELD_OUT_4_REQUESTS: list[tuple[str, list[str]]] = [ + ( + "Run a pathway enrichment on KEAP1, NFE2L2, HMOX1, NQO1 please", + ["KEAP1", "NFE2L2", "HMOX1", "NQO1"], + ), + ( + "perform ORA with the following:\nPINK1\nPRKN\nPARK7\nLRRK2\nSNCA", + ["PINK1", "PRKN", "PARK7", "LRRK2", "SNCA"], + ), + ( + "Could you do an over-representation analysis of GLS, GLUD1, GOT2, SLC1A5?", + ["GLS", "GLUD1", "GOT2", "SLC1A5"], + ), + ("analyse these: cd274, pdcd1, ctla4, lag3", ["cd274", "pdcd1", "ctla4", "lag3"]), + ( + "Enrichment analysis on SIRT1, SIRT3, PPARGC1A, FOXO3 - can you run it?", + ["SIRT1", "SIRT3", "PPARGC1A", "FOXO3"], + ), + ("run ORA on Q16539, P45983, P53779", ["Q16539", "P45983", "P53779"]), + ( + "Can we run an enrichment for ACE2, TMPRSS2, FURIN and CTSL", + ["ACE2", "TMPRSS2", "FURIN", "CTSL"], + ), + ( + "please do a GSEA with RB1, E2F1, CDK2, CCNE1, CDKN1B", + ["RB1", "E2F1", "CDK2", "CCNE1", "CDKN1B"], + ), + ( + "run pathway analysis on WNT3A FZD7 LRP6 DVL2 AXIN1", + ["WNT3A", "FZD7", "LRP6", "DVL2", "AXIN1"], + ), +] + +HELD_OUT_4_QUESTIONS: list[str] = [ + "What does KEAP1 do to NFE2L2 under oxidative stress?", + "Is PINK1, PRKN signalling part of mitophagy in Reactome?", + "Can you tell me if GLS and GLUD1 are in glutamine metabolism?", + "Why are CD274 and PDCD1 targets for immunotherapy?", + "Which pathway would SIRT1, SIRT3, FOXO3 all belong to?", + "Run a search for the ACE2 entry please", + "Can I use Ensembl IDs like ENSG00000130234 and ENSG00000184012 for analysis?", + "What is the difference between RB1 and CDKN1B?", + "Show me the reactions for PAX6 and SOX1 in neural development", + "How do WNT3A and FZD7 activate beta-catenin?", + "Where is LRRK2 located in the cell?", + "I ran an enrichment of SNCA, LRRK2, PARK7 last week - is the result still valid after the release?", +] + #: Current behaviour that is wrong, pinned so a change is noticed. KNOWN_LIMITS: list[tuple[str, list[str] | None]] = [ ( "I'd like an over-representation analysis on these genes - FOXP3, IL2RA, CTLA4, IKZF2", None, ), - ( - "Is my list of TP53, MDM2 targets enriched for apoptosis? I have not run anything yet, just asking", - ["TP53", "MDM2"], - ), ( "gene\tlog2FC\tpadj\nTP53\t2.1\t0.001\nMDM2\t1.5\t0.01\nCDKN1A\t3.2\t0.0001\nplease run an enrichment analysis", None, ), + ("find pathways for my genes: Pax6, Sox1, Nes, Otx2", None), ] diff --git a/tests/analysis/test_gene_list.py b/tests/analysis/test_gene_list.py index 2543cfa..f1568ea 100644 --- a/tests/analysis/test_gene_list.py +++ b/tests/analysis/test_gene_list.py @@ -44,6 +44,8 @@ *phrases.HELD_OUT_REQUESTS, *phrases.SECOND_REVIEW_REQUESTS, *phrases.HELD_OUT_3_REQUESTS, + *phrases.THIRD_REVIEW_REQUESTS, + *phrases.HELD_OUT_4_REQUESTS, *( (phrases.TRAILING_BASE + tail, ["TP53", "MDM2", "CDKN1A"]) for tail in phrases.TRAILING @@ -53,6 +55,16 @@ ["TP53", "MDM2", "MYC", "CCND1"], ), ("Run ORA on\r\nTP53\r\nMDM2", ["TP53", "MDM2"]), + # Round three: a lower-case word after a list is the sentence going on. + ("run ORA on TP53, MDM2, then show me the top hits", ["TP53", "MDM2"]), + ("run ORA on TP53, MDM2 or similar", ["TP53", "MDM2"]), + ("run an enrichment on TP53, MDM2, CDKN1A, cheers", ["TP53", "MDM2", "CDKN1A"]), + ( + "run an enrichment on TP53, MDM2, CDKN1A, and plot it", + ["TP53", "MDM2", "CDKN1A"], + ), + ("analyze my genes - CTNNB1, APC, AXIN2", ["CTNNB1", "APC", "AXIN2"]), + ("run enrichment in STAT1, STAT2, IRF9", ["STAT1", "STAT2", "IRF9"]), # A list can open the message, in lower case. ("egfr, kras, braf - run ORA on these", ["egfr", "kras", "braf"]), # The verb is not the list's first member. @@ -78,6 +90,8 @@ *phrases.HELD_OUT_QUESTIONS, *phrases.SECOND_REVIEW_QUESTIONS, *phrases.HELD_OUT_3_QUESTIONS, + *phrases.THIRD_REVIEW_QUESTIONS, + *phrases.HELD_OUT_4_QUESTIONS, "please run GSA on my matrix, columns are sample1, sample2, sample3", "Map EGFR mutations L858R and T790M to pathways", ] @@ -93,7 +107,10 @@ def test_known_limits_are_as_recorded(text: str, current: list[str] | None) -> N @pytest.mark.parametrize( "text", [ - "run ORA on:\n" + "\n".join(f"GENE{i}" for i in range(8000)), + # Under the cap, so the parser reads all of it: the first rewrite + # took 38 s on 4,000 lines. (8,000 lines are over the cap, and would + # pass on the old code unread.) + "run ORA on:\n" + "\n".join(f"GENE{i}" for i in range(6000)), "run ORA on TP53, MDM2" + "\n" * 50_000 + "x", "run ORA on TP53, MDM2" + "?" * 50_000 + "x", "run ORA on: TP53, MDM2" + " " * 50_000 + "!MYC", diff --git a/tests/analysis/test_proposals.py b/tests/analysis/test_proposals.py new file mode 100644 index 0000000..ab8c15d --- /dev/null +++ b/tests/analysis/test_proposals.py @@ -0,0 +1,86 @@ +"""Offers the chat has made and not yet had answered. + +Round three of review found the first version of this state crashed on +resume: it lived in Chainlit's user_session, which is saved as JSON, and the +button objects came back as null. +""" + +import json +from dataclasses import asdict + +from analysis.proposals import Proposal, ProposalStore + + +def offer(text: str = "run ORA on TP53, MDM2") -> Proposal: + return Proposal( + text=text, + message_id="m1", + identifiers=("TP53", "MDM2"), + actions=(("gene_list_run", "a1"), ("gene_list_no", "a2")), + ) + + +def test_an_offer_is_plain_data() -> None: + # Whatever holds it, it survives JSON unchanged -- the failure was + # cl.Action objects turning into null. + data = asdict(offer()) + assert json.loads(json.dumps(data)) == { + **data, + "identifiers": ["TP53", "MDM2"], + "actions": [["gene_list_run", "a1"], ["gene_list_no", "a2"]], + } + + +def test_an_offer_is_claimed_once() -> None: + store = ProposalStore() + store.put("s", "p", offer()) + assert store.take("s", "p") == offer() + assert store.take("s", "p") is None # double click, or click then "yes" + + +def test_offers_are_per_session() -> None: + store = ProposalStore() + store.put("s1", "p", offer()) + assert store.take("s2", "p") is None + assert store.take("s1", "p") is not None + + +def test_yes_means_only_the_offer_just_made() -> None: + store = ProposalStore() + store.put("s", "p1", offer()) + store.put("s", "p2", offer()) + assert store.take_latest("s") == "p2" + # Read once: the next message is no longer "straight after" the offer. + assert store.take_latest("s") is None + + +def test_clicking_the_latest_offer_ends_its_yes() -> None: + store = ProposalStore() + store.put("s", "p", offer()) + store.take("s", "p") + assert store.take_latest("s") is None + + +def test_older_offers_are_evicted_and_returned_for_their_buttons() -> None: + store = ProposalStore(max_per_session=2) + first = offer("first") + store.put("s", "p1", first) + store.put("s", "p2", offer("second")) + assert store.put("s", "p3", offer("third")) == [first] + assert store.take("s", "p1") is None + assert store.take("s", "p3") is not None + + +def test_sessions_are_bounded() -> None: + store = ProposalStore(max_sessions=2) + for name in ("a", "b", "c"): + store.put(name, "p", offer()) + assert store.take("a", "p") is None + assert store.take("c", "p") is not None + + +def test_nothing_to_take_is_none_not_an_error() -> None: + store = ProposalStore() + assert store.take("unknown", "p") is None + assert store.take("unknown", None) is None + assert store.take_latest("unknown") is None