diff --git a/.chainlit/config.toml b/.chainlit/config.toml index b3c33cec..4e2f3e74 100644 --- a/.chainlit/config.toml +++ b/.chainlit/config.toml @@ -33,7 +33,7 @@ edit_message = true # Authorize users to spontaneously upload files with messages [features.spontaneous_file_upload] - enabled = false + enabled = true # Define accepted file types using MIME types # Examples: # 1. For specific file types: @@ -43,9 +43,20 @@ edit_message = true # 3. For specific file extensions: # accept = { "application/octet-stream" = [".xyz", ".pdb"] } # Note: Using "*/*" is not recommended as it may cause browser warnings - accept = ["*/*"] - max_files = 20 - max_size_mb = 500 + # Expression matrices only. "*/*" is what the template ships and it + # invites every other kind of file into a chat that can do nothing with + # them. + accept = { "text/tab-separated-values" = [".tsv", ".txt"], "text/csv" = [".csv"] } + max_files = 1 + # 20, not the template's 500. This host has one 88 GB volume that sits + # around 90-95% full, and ~/update-beta-chat.sh refuses to deploy under + # 6 GB free -- so a handful of 500 MB uploads would take the chat down + # and block the fix. A 16-sample expression matrix is 1.2 MB. + # + # src/gsa/upload.py enforces the same ceiling again, because this one is + # a client-side convenience: Chainlit is what stops the browser sending + # it, not what stops the server accepting it. + max_size_mb = 20 [features.audio] # Sample rate of the audio diff --git a/bin/chat-chainlit.py b/bin/chat-chainlit.py index 7a848911..9acee06b 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 +from pathlib import Path import chainlit as cl from chainlit.data.base import BaseDataLayer @@ -16,6 +17,7 @@ from agent.profiles import get_chat_profiles from agent.profiles.base import OutputState from agent.registry import get_graph +from gsa.chainlit_flow import Attachment, matrix_attachment, run_analysis from util.chainlit_helpers import ( PrefixedS3StorageClient, is_feature_enabled, @@ -139,6 +141,52 @@ async def end() -> None: await static_messages(config, TriggerEvent.on_chat_end) +async def run_gsa_analysis(attachment: Attachment) -> None: + """Drive `gsa.chainlit_flow` with this session's chat operations. + + The flow takes these four as arguments so it can be tested without a + browser; this is the only place that knows they are Chainlit. + """ + progress = cl.Message(content="Reading your file…") + await progress.send() + + async def ask_for_grouping() -> str | None: + answer = await cl.AskUserMessage( + content="Which group is each sample in?", timeout=600 + ).send() + return (answer or {}).get("output") if answer else None + + async def send(text: str) -> None: + await cl.Message(content=text).send() + + async def update_progress(text: str) -> None: + progress.content = text + await progress.update() + + async def send_file(path: Path) -> None: + await cl.Message( + content="", + elements=[cl.File(name=path.name, path=str(path), display="inline")], + ).send() + + # The progress line is removed rather than marked "Done". + # + # A `finally` that sets "Done." runs on the failure paths too, so a user + # whose analysis died would have been told it finished, one line above + # the message explaining that it had not. `run_analysis` says what + # happened on every path; this only has to stop the spinner. + try: + await run_analysis( + attachment, + ask_for_grouping=ask_for_grouping, + send=send, + update_progress=update_progress, + send_file=send_file, + ) + finally: + await progress.remove() + + @cl.on_message async def main(message: cl.Message) -> None: if await message_rate_limited(config): @@ -146,6 +194,17 @@ async def main(message: cl.Message) -> None: 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 + message_count: int = cl.user_session.get("message_count", 0) + 1 cl.user_session.set("message_count", message_count) diff --git a/specs/012-run-gene-analysis/spec.md b/specs/012-run-gene-analysis/spec.md index 8a1d2b7c..e7146012 100644 --- a/specs/012-run-gene-analysis/spec.md +++ b/specs/012-run-gene-analysis/spec.md @@ -4,7 +4,7 @@ **Created**: 2026-09-21 -**Status**: Draft +**Status**: Implemented for uploads; public-dataset path deferred **Input**: User description: "Run a gene set analysis from an uploaded expression matrix in the chat, and return the pathway results as a downloadable file" @@ -153,3 +153,30 @@ and nothing has been sent to the model. - Whether an analysis should survive a chat session ending. Results live behind an analysis ID at the service, so resuming is possible; whether it is wanted is a product question. + +--- + +## What shipped, 2026-09-22 + +**User Story 2 (upload) and Story 3 (results without a summary) are done.** +Story 1 (public datasets by identifier) is built underneath -- +`submit_public_dataset` works and is tested -- but has no chat route yet, +because reaching it needs the model to choose a dataset and a factor, which +is a conversation design rather than a plumbing job. + +Shipping Story 2 before Story 1 inverts the spec's priorities. The reason is +that the upload path turned out to be the *simpler* one: a file arrives with +its own sample names, so there is nothing to search for and nothing to +disambiguate. + +**Nothing reaches OpenAI.** Story 3 was written as "a user who declines the +summary still gets their results". What shipped is that with no summary at +all: the result is described from the table and attached as a file, both of +which are the user's own data returning to the user. `Finished.for_model` +is computed and sent nowhere. When a summary is added it must be given the +bounded allow-listed view, behind the existing warning -- not the result. + +**FR-006 landed in two places on purpose.** `.chainlit/config.toml` caps +uploads at 20 MB, and `src/gsa/upload.py` caps them again. The first stops +a browser sending the file; the second stops the server accepting it. Only +the second is a guarantee. diff --git a/src/gsa/chainlit_flow.py b/src/gsa/chainlit_flow.py new file mode 100644 index 00000000..705f0adc --- /dev/null +++ b/src/gsa/chainlit_flow.py @@ -0,0 +1,202 @@ +"""The Chainlit side of running a gene set analysis. + +Thin on purpose. Every decision -- what a usable reply is, what the person +is told, what the model may see -- lives in `gsa.chat` and is tested there. +What is left here is the part a browser has to verify: reading an attached +file, asking a follow-up question, streaming progress, sending a download. + +**Why this is not a tool the agent calls.** The run takes minutes, which is +longer than a chat turn, and the matrix is 1.2 MB, which must never enter +the model's context. A tool call would put the model in the middle of both +problems. So an attached file routes here *instead of* the graph, and the +model is handed only the bounded result at the end, if the user asks for a +summary at all. +""" + +from pathlib import Path +from typing import Any, Protocol + +from gsa import chat +from gsa.client import GsaClient +from gsa.job import AnalysisFailedError, await_result, submit_uploaded_matrix +from gsa.upload import UploadRejectedError, discard, validate +from util.logging import logging + +logger = logging.getLogger(__name__) + +#: Where result tables are written. Bounded by `prune_results` on each run. +#: +#: Not a bare path under a world-writable `/tmp`: these are a user's own +#: analysis results, and on a shared host anyone could pre-create the +#: directory or replace a file in it. `results_dir()` creates it 0700 and +#: owned by this process, and `GSA_RESULTS_DIR` lets a deployment put it +#: somewhere with a real quota. +RESULTS_DIR_ENV = "GSA_RESULTS_DIR" + + +def results_dir() -> Path: + """The results directory, created private to this user.""" + import os + import tempfile + + configured = os.environ.get(RESULTS_DIR_ENV) + base = ( + Path(configured) if configured else Path(tempfile.gettempdir()) / "reactome-gsa" + ) + base.mkdir(parents=True, exist_ok=True, mode=0o700) + return base + + +#: Extensions we will try to read as a matrix. Anything else attached is +#: almost certainly meant for a different conversation. +MATRIX_SUFFIXES = {".tsv", ".csv", ".txt"} + + +class Attachment(Protocol): + """The part of a Chainlit file element this needs.""" + + name: str + path: str + + +def matrix_attachment(elements: list[Any] | None) -> Attachment | None: + """The first attachment that could be an expression matrix, if any. + + Returns None for a message with no attachments, which is every ordinary + message -- so the caller can use this as the routing decision without + knowing anything about Chainlit. + """ + for element in elements or []: + name = getattr(element, "name", "") or "" + path = getattr(element, "path", None) + if path and Path(name).suffix.lower() in MATRIX_SUFFIXES: + found: Attachment = element + return found + return None + + +async def run_analysis( + attachment: Attachment, + *, + ask_for_grouping: Any, + send: Any, + update_progress: Any, + send_file: Any, + client: GsaClient | None = None, +) -> None: + """Validate, ask for groups, submit, poll, deliver. + + The callables are passed in rather than imported so this can be driven + without Chainlit. They are the four things a chat has to be able to do: + ask a question and wait, say something, revise what was said, and hand + over a file. + """ + path = Path(attachment.path) + try: + await _run( + attachment, path, ask_for_grouping, send, update_progress, send_file, client + ) + finally: + # The upload does not survive this function, whatever happened. + # + # Each path used to delete it for itself, which covered the ones I + # thought of. It did not cover a file Chainlit had not finished + # writing (`FileNotFoundError` straight out of `validate`), or an + # `AskUserMessage` that times out or is interrupted -- and an + # unhandled error is exactly when nobody is around to tidy up. + # `discard` is safe to call twice, so the paths that already delete + # it can keep doing so. + discard(path) + + +async def _run( + attachment: Attachment, + path: Path, + ask_for_grouping: Any, + send: Any, + update_progress: Any, + send_file: Any, + client: GsaClient | None, +) -> None: + try: + matrix = validate(path) + except UploadRejectedError as refusal: + await send(str(refusal)) + return + except OSError: + # Unreadable, vanished, or not a file. The user did nothing wrong + # that they can act on, so do not describe it as their mistake. + logger.exception("could not read an uploaded file") + await send("I could not read that file — please try attaching it again.") + return + + await send(chat.describe_matrix(matrix)) + + reply = await ask_for_grouping() + if not reply: + discard(path) + await send( + "No labels arrived, so I have not run anything. The file is deleted." + ) + return + + try: + grouping = chat.parse_grouping(reply, len(matrix.samples)) + except chat.ReplyUnusableError as unusable: + await send(f"{unusable} Send the file again when you are ready.") + return + + gsa = client or GsaClient() + try: + # `submit_uploaded_matrix` deletes the file itself, on every path. + analysis_id = await submit_uploaded_matrix( + gsa, + matrix=matrix, + dataset_type="rnaseq_counts", + analysis_group=grouping.labels, + group1=grouping.group1, + group2=grouping.group2, + ) + except AnalysisFailedError as failure: + await send(f"I could not start the analysis: {failure}") + return + except Exception: + logger.exception("gsa submission failed") + await send("I could not reach the analysis service. Nothing was run.") + return + + await send( + f"Started. This usually takes a few minutes — " + f"comparing **{grouping.group1}** with **{grouping.group2}**." + ) + + try: + finished = await await_result( + gsa, + analysis_id, + out_dir=results_dir(), + on_progress=lambda status: update_progress(chat.describe_progress(status)), + ) + except AnalysisFailedError as failure: + # The service accepts and then fails, and says so only here. + await send(f"The analysis did not finish: {failure}") + return + except Exception: + logger.exception("gsa analysis failed", extra={"analysis": analysis_id}) + await send("Something went wrong while waiting for the analysis.") + return + + # `finished.for_model` is deliberately not used here. + # + # Nothing about this analysis reaches OpenAI. The result is described + # from the table and handed over as a file, both of which are the + # user's own data going back to the user. A model-written summary is + # spec 012's Story 3 the other way round -- opt-in, behind the existing + # warning -- and until that exists, the honest state is that the + # allow-listed view is computed and sent nowhere. + # + # It is computed rather than skipped so the disclosure rules stay + # exercised by the tests; if a summary is added later, the bounded view + # is what it must be given, not the result. + await send(chat.describe_result(finished)) + await send_file(finished.table_path) diff --git a/src/gsa/chat.py b/src/gsa/chat.py new file mode 100644 index 00000000..14703717 --- /dev/null +++ b/src/gsa/chat.py @@ -0,0 +1,158 @@ +"""Turning a chat exchange into an analysis, and back into words. + +Kept apart from the Chainlit handler on purpose. Everything here is a pure +function over plain values, so the decisions -- what counts as a usable +reply, what the user is told, what the model is allowed to see -- can be +tested. The handler that remains is wiring, and wiring is what a browser is +for. + +**The audience split is the point.** A finished analysis produces three +things and they go to three different places: a bounded allow-listed view +for the model, a Pathway Browser link for the person, and a full table as a +file. `describe_result` writes what the person reads; it never becomes a +prompt, and `Finished.for_model` never becomes a message. +""" + +from dataclasses import dataclass + +from gsa.client import AnalysisStatus +from gsa.job import Finished +from gsa.upload import Matrix + +#: Shown when a matrix arrives, before anything is submitted. +MAX_SAMPLES_LISTED = 24 + + +class ReplyUnusableError(ValueError): + """The reply cannot be turned into a grouping, with a reason to show.""" + + +@dataclass(frozen=True) +class Grouping: + """One label per sample, and the two labels to compare.""" + + labels: list[str] + group1: str + group2: str + + +def describe_matrix(matrix: Matrix) -> str: + """What the user is told about the file they just sent. + + Their sample names are echoed back deliberately -- they need to see + what was read in order to label it, and it is their own data being + shown to them. It is the *model* that never sees these. + """ + shown = matrix.samples[:MAX_SAMPLES_LISTED] + more = len(matrix.samples) - len(shown) + genes = ( + "an unknown number of" + if matrix.gene_count is None + else f"{matrix.gene_count:,}" + ) + + lines = [ + f"Read **{len(matrix.samples)} samples** and {genes} genes " + f"({matrix.size_bytes / 1e6:.1f} MB).", + "", + "Samples, in column order:", + "", + " " + + ", ".join(f"`{name}`" for name in shown) + + (f" …and {more} more" if more else ""), + "", + "To run the analysis I need to know which group each sample belongs " + "to. Reply with one label per sample, in that order, separated by " + "commas — for example `control, control, treated, treated`.", + ] + return "\n".join(lines) + + +def parse_grouping(reply: str, sample_count: int) -> Grouping: + """Turn a reply into a grouping, or say why it cannot be one. + + Deliberately forgiving about separators and case, and strict about the + count: a label list that does not line up with the columns produces an + analysis of the wrong thing, which is worse than a refusal because it + returns a plausible answer. + """ + labels = [ + part.strip() for part in reply.replace("\t", ",").replace(";", ",").split(",") + ] + labels = [label for label in labels if label] + + if not labels: + raise ReplyUnusableError("I could not find any group labels in that.") + + if len(labels) != sample_count: + raise ReplyUnusableError( + f"That is {len(labels)} label{'s' if len(labels) != 1 else ''} for " + f"{sample_count} samples. I need exactly one per sample, in the " + f"order the columns appear." + ) + + # Case-insensitive grouping, but the user's own spelling is kept: the + # labels go to the service and come back in its output, and silently + # lower-casing someone's "Treated" makes the result harder to read + # against their own notes. + seen: dict[str, str] = {} + for label in labels: + seen.setdefault(label.casefold(), label) + + if len(seen) < 2: + raise ReplyUnusableError( + "All the samples have the same label, so there is nothing to " + "compare. A gene set analysis needs two groups." + ) + if len(seen) > 2: + names = ", ".join(sorted(seen.values())) + raise ReplyUnusableError( + f"I found more than two groups ({names}). This runs one " + f"comparison at a time, so please use exactly two labels." + ) + + canonical = [seen[label.casefold()] for label in labels] + group1, group2 = sorted(seen.values()) + return Grouping(labels=canonical, group1=group1, group2=group2) + + +def describe_progress(status: AnalysisStatus) -> str: + """One line, safe to send repeatedly as an edit.""" + percent = max(0, min(100, int(status.completed * 100))) + detail = status.description.strip() or "working" + return f"Running the analysis — {percent}% · {detail}" + + +def describe_result(finished: Finished) -> str: + """What the person reads. Never a prompt. + + The Pathway Browser link is included *here* and not in anything the + model sees: its URL carries the analysis token, and whoever holds that + can fetch the unredacted result back from the service. + """ + view = finished.for_model + total = view.get("pathway_count", 0) + significant = view.get("significant_count", 0) + top = view.get("top_pathways") or [] + + lines = [ + f"**{significant:,} of {total:,} pathways** are significant at FDR < 0.05.", + "", + ] + if top: + lines += [ + "| Pathway | Direction | FDR |", + "|---|---|---|", + ] + for pathway in top[:10]: + lines.append( + f"| {pathway['name']} | {pathway['direction']} | {pathway['fdr']:.2g} |" + ) + lines.append("") + + for name, url in finished.links: + lines.append(f"[{name}]({url})") + if finished.links: + lines.append("") + lines.append("The full table, with every column, is attached.") + return "\n".join(lines) diff --git a/src/gsa/job.py b/src/gsa/job.py index b1b6589d..60baef2e 100644 --- a/src/gsa/job.py +++ b/src/gsa/job.py @@ -17,6 +17,7 @@ from collections.abc import Awaitable, Callable from dataclasses import dataclass from pathlib import Path +from typing import Any from gsa import results as gsa_results from gsa.client import AnalysisStatus, GsaClient, GsaError @@ -72,7 +73,7 @@ class Finished: """ analysis_id: str - for_model: dict[str, object] + for_model: dict[str, Any] links: list[tuple[str, str]] table_path: Path diff --git a/tests/gsa/test_gsa_chainlit_flow.py b/tests/gsa/test_gsa_chainlit_flow.py new file mode 100644 index 00000000..ba498b98 --- /dev/null +++ b/tests/gsa/test_gsa_chainlit_flow.py @@ -0,0 +1,270 @@ +"""The flow, driven without a browser. + +`run_analysis` takes its four chat operations as arguments, so the paths +that matter -- a refused file, an unusable reply, a submission that fails, +an analysis that fails after being accepted -- can be exercised here. What +is left for a browser is whether Chainlit's own callbacks are wired to the +right arguments. +""" + +import asyncio +import functools +import json +import os +from pathlib import Path +from typing import Any + +import pytest + +from gsa import chainlit_flow +from gsa.client import AnalysisStatus +from gsa.job import AnalysisFailedError + +FIXTURE = Path(__file__).parent / "result_fixture.json" + + +def asyncio_test(fn: Any) -> Any: + @functools.wraps(fn) + def wrapper(*args: Any, **kwargs: Any) -> Any: + return asyncio.run(fn(*args, **kwargs)) + + return wrapper + + +class Chat: + """Records what a user would have seen.""" + + def __init__(self, reply: str | None = "a, a, b, b") -> None: + self.reply = reply + self.said: list[str] = [] + self.progress: list[str] = [] + self.files: list[Path] = [] + + async def ask(self) -> str | None: + return self.reply + + async def send(self, text: str) -> None: + self.said.append(text) + + async def update(self, text: str) -> None: + self.progress.append(text) + + async def send_file(self, path: Path) -> None: + self.files.append(path) + + @property + def transcript(self) -> str: + return "\n".join(self.said) + + +class StubClient: + def __init__(self, *, fail_submit: bool = False, fail_run: bool = False) -> None: + self.fail_submit = fail_submit + self.fail_run = fail_run + self.submitted: dict[str, Any] = {} + + async def submit(self, **kwargs: Any) -> str: + if self.fail_submit: + raise AnalysisFailedError("the service refused it") + self.submitted = kwargs + return "an-1" + + async def analysis_status(self, analysis_id: str) -> AnalysisStatus: + if self.fail_run: + return AnalysisStatus("failed", "CONNECTION_FORCED - broker closed", 1.0) + return AnalysisStatus("complete", "Analysis done", 1.0) + + async def result(self, analysis_id: str) -> dict[str, Any]: + loaded: dict[str, Any] = json.loads(FIXTURE.read_text()) + return loaded + + +class Attached: + def __init__(self, path: Path) -> None: + self.name = path.name + self.path = str(path) + + +def a_matrix(tmp_path: Path, name: str = "counts.tsv") -> Attached: + path = tmp_path / name + path.write_text("\tS1\tS2\tS3\tS4\nENSG1\t1\t2\t3\t4\nENSG2\t5\t6\t7\t8\n") + return Attached(path) + + +async def run(attachment: Attached, chat_: Chat, client: Any, out: Path) -> None: + os.environ[chainlit_flow.RESULTS_DIR_ENV] = str(out) + await chainlit_flow.run_analysis( + attachment, + ask_for_grouping=chat_.ask, + send=chat_.send, + update_progress=chat_.update, + send_file=chat_.send_file, + client=client, + ) + + +class TestRouting: + def test_an_ordinary_message_is_not_an_analysis(self) -> None: + # Every normal message goes through here, so it must say no to the + # common case without knowing anything about Chainlit. + assert chainlit_flow.matrix_attachment(None) is None + assert chainlit_flow.matrix_attachment([]) is None + + def test_a_pdf_is_not_a_matrix(self, tmp_path: Path) -> None: + pdf = tmp_path / "paper.pdf" + pdf.write_text("x") + assert chainlit_flow.matrix_attachment([Attached(pdf)]) is None + + def test_a_tsv_is(self, tmp_path: Path) -> None: + assert chainlit_flow.matrix_attachment([a_matrix(tmp_path)]) is not None + + +class TestTheUnhappyPaths: + @asyncio_test + async def test_a_rejected_file_is_explained_and_deleted( + self, tmp_path: Path + ) -> None: + bad = tmp_path / "notes.txt" + bad.write_text("just some prose\n") + chat_ = Chat() + + await run(Attached(bad), chat_, StubClient(), tmp_path / "out") + + assert "expression matrix" in chat_.transcript + assert not bad.exists() + assert not chat_.files + + @asyncio_test + async def test_an_unusable_reply_deletes_the_file_too(self, tmp_path: Path) -> None: + # The count mismatch is the likeliest mistake, and the file must + # not survive it -- this host has 5 GB free. + attachment = a_matrix(tmp_path) + chat_ = Chat(reply="a, b") + + await run(attachment, chat_, StubClient(), tmp_path / "out") + + assert "one per sample" in chat_.transcript + assert not Path(attachment.path).exists() + + @asyncio_test + async def test_no_reply_at_all_deletes_the_file(self, tmp_path: Path) -> None: + attachment = a_matrix(tmp_path) + chat_ = Chat(reply=None) + + await run(attachment, chat_, StubClient(), tmp_path / "out") + + assert "not run anything" in chat_.transcript + assert not Path(attachment.path).exists() + + @asyncio_test + async def test_a_failed_submission_is_reported(self, tmp_path: Path) -> None: + chat_ = Chat() + await run( + a_matrix(tmp_path), chat_, StubClient(fail_submit=True), tmp_path / "out" + ) + + assert "could not start" in chat_.transcript + assert not chat_.files + + @asyncio_test + async def test_an_analysis_that_fails_after_starting_is_reported( + self, tmp_path: Path + ) -> None: + # The measured failure: accepted with a 200, then dead. A flow that + # only checked the submission would sit waiting forever, or claim + # success. + chat_ = Chat() + await run( + a_matrix(tmp_path), chat_, StubClient(fail_run=True), tmp_path / "out" + ) + + assert "did not finish" in chat_.transcript + assert "CONNECTION_FORCED" in chat_.transcript + assert not chat_.files + + +class TestTheHappyPath: + @asyncio_test + async def test_it_delivers_a_summary_and_a_file(self, tmp_path: Path) -> None: + chat_ = Chat() + client = StubClient() + + await run(a_matrix(tmp_path), chat_, client, tmp_path / "out") + + assert "significant" in chat_.transcript + assert chat_.files, "the user must get their table" + assert chat_.files[0].exists() + assert "MeanWeightT0" in chat_.files[0].read_text() + + @asyncio_test + async def test_the_filename_never_reaches_the_service(self, tmp_path: Path) -> None: + chat_ = Chat() + client = StubClient() + + await run( + a_matrix(tmp_path, "smith_lab_unpublished_2026.tsv"), + chat_, + client, + tmp_path / "out", + ) + + assert "smith_lab" not in json.dumps(client.submitted.get("dataset_name")) + + @asyncio_test + async def test_the_uploaded_matrix_is_gone_afterwards(self, tmp_path: Path) -> None: + attachment = a_matrix(tmp_path) + await run(attachment, Chat(), StubClient(), tmp_path / "out") + assert not Path(attachment.path).exists() + + +class TestTheUploadNeverSurvives: + """The file is gone when this function returns, whatever happened. + + Each branch used to delete it for itself, which covered the cases I had + thought of and not the ones I had not: a file Chainlit has not finished + writing, or an `AskUserMessage` that times out. An unhandled error is + precisely when nobody is around to tidy up, and this host has 5 GB free. + """ + + @asyncio_test + async def test_a_file_that_is_not_there_is_reported_not_raised( + self, tmp_path: Path + ) -> None: + missing = Attached(tmp_path / "never-written.tsv") + chat_ = Chat() + + await run(missing, chat_, StubClient(), tmp_path / "out") + + assert "could not read that file" in chat_.transcript + # And it does not describe it as the user's mistake, because it is + # not one they can act on. + assert "expression matrix" not in chat_.transcript + + @asyncio_test + async def test_an_exception_from_the_question_still_deletes_it( + self, tmp_path: Path + ) -> None: + attachment = a_matrix(tmp_path) + + class Exploding(Chat): + async def ask(self) -> str | None: + raise TimeoutError("the user never answered") + + with pytest.raises(TimeoutError): + await run(attachment, Exploding(), StubClient(), tmp_path / "out") + + assert not Path(attachment.path).exists() + + @asyncio_test + async def test_a_binary_file_is_refused_rather_than_crashing( + self, tmp_path: Path + ) -> None: + # Somebody will attach a spreadsheet or an image with a .tsv name. + path = tmp_path / "image.tsv" + path.write_bytes(bytes(range(256)) * 50) + chat_ = Chat() + + await run(Attached(path), chat_, StubClient(), tmp_path / "out") + + assert chat_.said, "it must say something rather than fail silently" + assert not path.exists() diff --git a/tests/gsa/test_gsa_chat.py b/tests/gsa/test_gsa_chat.py new file mode 100644 index 00000000..332f9f55 --- /dev/null +++ b/tests/gsa/test_gsa_chat.py @@ -0,0 +1,143 @@ +"""What the user is asked, what they are told, and what the model is not. + +These are the decisions in the chat flow. The Chainlit handler around them +is wiring; a browser tests wiring, and nothing tests a decision that only +exists inside a UI callback. +""" + +import json +import tempfile +from pathlib import Path + +import pytest + +from gsa import chat +from gsa.client import AnalysisStatus +from gsa.job import Finished +from gsa.upload import Matrix + + +def matrix(samples: list[str], genes: int | None = 12) -> Matrix: + return Matrix( + path=Path(tempfile.gettempdir()) / "x.tsv", + size_bytes=1_200_000, + samples=samples, + gene_count=genes, + ) + + +class TestParsingTheReply: + def test_accepts_a_plain_comma_list(self) -> None: + grouping = chat.parse_grouping("control, control, treated, treated", 4) + assert grouping.labels == ["control", "control", "treated", "treated"] + assert (grouping.group1, grouping.group2) == ("control", "treated") + + @pytest.mark.parametrize("reply", ["a;a;b;b", "a\ta\tb\tb", "a , a ,b, b"]) + def test_is_forgiving_about_separators_and_spacing(self, reply: str) -> None: + assert chat.parse_grouping(reply, 4).labels == ["a", "a", "b", "b"] + + def test_groups_case_insensitively_but_keeps_the_spelling(self) -> None: + # The labels reach the service and come back in its output, so + # silently lower-casing someone's "Treated" makes the result harder + # to read against their own notes. + grouping = chat.parse_grouping("Treated, treated, Control, control", 4) + assert grouping.labels == ["Treated", "Treated", "Control", "Control"] + assert {grouping.group1, grouping.group2} == {"Treated", "Control"} + + def test_refuses_a_count_that_does_not_match(self) -> None: + # The dangerous one. Three labels for four columns still analyses + # *something*, and returns a plausible answer to the wrong question. + with pytest.raises(chat.ReplyUnusableError, match="one per sample"): + chat.parse_grouping("a, b, c", 4) + + def test_refuses_one_group(self) -> None: + with pytest.raises(chat.ReplyUnusableError, match="nothing to compare"): + chat.parse_grouping("treated, treated", 2) + + def test_refuses_three_groups_and_names_them(self) -> None: + with pytest.raises(chat.ReplyUnusableError, match="a, b, c"): + chat.parse_grouping("a, b, c", 3) + + def test_refuses_an_empty_reply(self) -> None: + with pytest.raises(chat.ReplyUnusableError, match="could not find"): + chat.parse_grouping(" ,, ", 4) + + +class TestWhatTheUserIsTold: + def test_the_sample_names_are_echoed_back(self) -> None: + # They have to see what was read in order to label it, and it is + # their own data being shown to them. + text = chat.describe_matrix(matrix(["Ctrl_1", "Ctrl_2", "Tr_1"])) + assert "Ctrl_1" in text + assert "3 samples" in text + + def test_a_long_sample_list_is_truncated(self) -> None: + text = chat.describe_matrix(matrix([f"S{i}" for i in range(50)])) + assert "and 26 more" in text + assert "50 samples" in text + + def test_an_uncounted_gene_total_is_said_in_words(self) -> None: + # `gene_count` is None for a file too long to finish counting. It + # must not render as "None genes" or "-1 genes". + text = chat.describe_matrix(matrix(["A", "B"], genes=None)) + assert "unknown number" in text + assert "None" not in text + + def test_progress_is_a_single_clamped_line(self) -> None: + line = chat.describe_progress( + AnalysisStatus("running", "Permutation 900 / 1000", 0.9) + ) + assert "90%" in line + assert "\n" not in line + # The service has reported completion values outside 0..1. + assert "100%" in chat.describe_progress(AnalysisStatus("running", "x", 4.2)) + assert "0%" in chat.describe_progress(AnalysisStatus("running", "x", -1.0)) + + +def finished(tmp_path: Path) -> Finished: + table = tmp_path / "t.tsv" + table.write_text("Pathway\tName\n") + return Finished( + analysis_id="an-1", + for_model={ + "no_result": False, + "pathway_count": 2679, + "significant_count": 412, + "top_pathways": [ + { + "stId": "R-HSA-1", + "name": "Hemostasis", + "direction": "Up", + "fdr": 1e-5, + "genes": 6, + } + ], + }, + links=[ + ( + "Gene Set Analysis Summary", + "https://reactome.org/PathwayBrowser/#/ANALYSIS=TOKEN", + ) + ], + table_path=table, + ) + + +class TestWhatTheUserReads: + def test_it_reports_exact_counts_and_the_top_pathways(self, tmp_path: Path) -> None: + text = chat.describe_result(finished(tmp_path)) + assert "412" in text + assert "2,679" in text + assert "Hemostasis" in text + + def test_it_includes_the_pathway_browser_link(self, tmp_path: Path) -> None: + # The person gets the link. This is the half that must be present. + assert "PathwayBrowser" in chat.describe_result(finished(tmp_path)) + + def test_the_link_is_still_absent_from_what_the_model_sees( + self, tmp_path: Path + ) -> None: + # And this is the half that must not. The URL carries the analysis + # token, and whoever holds it can fetch the unredacted result -- + # including the user's own gene identifiers. + assert "PathwayBrowser" not in json.dumps(finished(tmp_path).for_model)