Explain the statistics with the reader's own numbers

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

@@ -65,13 +65,24 @@ count is reported instead; the reader knows what they submitted.
 
 ## Phase 5: User Story 3 — What do these numbers mean? (P3)
 
+**Fragility is computed in code, not asked for in the prompt.** A hit resting
+on fewer than five matched entities is flagged per pathway, because a model
+handed a tiny p-value and a tiny count describes the p-value. The threshold is
+on the *count*, not the ratio: 2 of 3 looks perfect by ratio and is nearly
+meaningless, 40 of 200 looks poor and is real evidence.
+
+**No per-pathway request parameter was added.** US3's independent test implies
+one, but the website has not asked for it and every shown pathway already
+carries its own counts, so the explanation is per-pathway without new contract
+surface.
+
 **Goal**: The statistics are explained using the reader's own numbers.
 
 **Independent test**: Ask about one pathway in a result; the explanation uses that pathway's counts, not generic definitions.
 
-- [ ] T021 [US3] Carry per-pathway counts into the prompt input for a named pathway in `src/analysis/summarise.py`
-- [ ] T022 [P] [US3] Test that a pathway significant by p-value but not by FDR is described as distinguishing the two, in `tests/analysis/test_summarise.py` (spec US3 scenario 2)
-- [ ] T023 [P] [US3] Test that a pathway with very few found entities is described as fragile, using its actual counts, in `tests/analysis/test_summarise.py` (spec US3 scenario 1)
+- [x] T021 [US3] Carry per-pathway counts into the prompt input for a named pathway in `src/analysis/summarise.py`
+- [x] T022 [P] [US3] Test that a pathway significant by p-value but not by FDR is described as distinguishing the two, in `tests/analysis/test_summarise.py` (spec US3 scenario 2)
+- [x] T023 [P] [US3] Test that a pathway with very few found entities is described as fragile, using its actual counts, in `tests/analysis/test_summarise.py` (spec US3 scenario 1)
 
 ## Phase 6: User Story 4 — Readings specific to the analysis type (P4)
 

src/analysis/summarise.py

@@ -15,11 +15,27 @@
 FDR_THRESHOLD = 0.05
 
 
+#: Below this many matched entities, a pathway's p-value rests on so little
+#: that it should not be read as evidence however small it is. Reactome's own
+#: user guide makes this point and it is the single thing a reader most often
+#: gets wrong -- a pathway with 2 of 3 entities found looks like a perfect hit
+#: and is nearly meaningless.
+#:
+#: Computed here rather than left to the model. Handed a small p-value and a
+#: small count and asked to be careful, a model describes the p-value.
+FRAGILE_BELOW_FOUND = 5
+
+
 def _significant(pathway: dict[str, Any]) -> bool:
     fdr = pathway.get("entities", {}).get("fdr")
     return isinstance(fdr, int | float) and fdr <= FDR_THRESHOLD
 
 
+def _fragile(pathway: dict[str, Any]) -> bool:
+    found = pathway.get("entities", {}).get("found")
+    return isinstance(found, int) and found < FRAGILE_BELOW_FOUND
+
+
 def prompt_input(payload: dict[str, Any]) -> dict[str, Any]:
     """What to tell the model, from the aggregate payload.
 
@@ -77,6 +93,9 @@ def prompt_input(payload: dict[str, Any]) -> dict[str, Any]:
                 "p_value": p.get("entities", {}).get("pValue"),
                 "fdr": p.get("entities", {}).get("fdr"),
                 "significant": _significant(p),
+                # True when the hit rests on too few entities to be evidence,
+                # whatever the p-value says.
+                "fragile": _fragile(p),
             }
             for p in pathways
         ],
@@ -119,6 +138,23 @@ def prompt_input(payload: dict[str, Any]) -> dict[str, Any]:
     "suggests the wrong species was analysed."
 )
 
+#: Always appended. Both halves are things a model will otherwise get wrong
+#: in the same direction -- towards overstating a finding.
+STATISTICS_INSTRUCTION = (
+    "When you call a pathway significant, say whether that is before or after "
+    "multiple-testing correction; `significant` in the data is after. A "
+    "pathway with a small p-value that does not pass correction is not a "
+    "finding, and saying so is more useful than omitting it. "
+    "Where `fragile` is true the hit rests on fewer than "
+    f"{FRAGILE_BELOW_FOUND} matched entities. Explain that in the reader's "
+    "terms using that pathway's own found and total counts -- a small p-value "
+    "on three entities is not evidence however small it is. **Never use the "
+    "word 'fragile' or any other field name from the data**: these are "
+    "internal labels, and a reader should be told what the counts mean, not "
+    "what we called it. Explain what the numbers mean for this result, never "
+    "in general."
+)
+
 #: Appended only when the reader chose the disclosing tier and unmatched
 #: identifiers were actually retrieved.
 #:

src/api/analysis_summary.py

@@ -31,6 +31,7 @@
 from analysis.summarise import (
     INEXACT_COUNT_INSTRUCTION,
     NAMED_UNMATCHED_INSTRUCTION,
+    STATISTICS_INSTRUCTION,
     UNMATCHED_INSTRUCTION,
     VERDICT_INSTRUCTION,
     prompt_input,
@@ -201,6 +202,7 @@ async def stream() -> AsyncIterator[str]:
                 instruction = VERDICT_INSTRUCTION[model_input["verdict"]]
                 if not model_input["significant_count_is_exact"]:
                     instruction = f"{instruction} {INEXACT_COUNT_INSTRUCTION}"
+                instruction = f"{instruction} {STATISTICS_INSTRUCTION}"
                 instruction = f"{instruction} {UNMATCHED_INSTRUCTION}"
                 if model_input.get("identifiers_not_found_names"):
                     instruction = f"{instruction} {NAMED_UNMATCHED_INSTRUCTION}"

tests/analysis/test_summarise.py

@@ -1,9 +1,11 @@
 """What the model is told about a result, before any model is involved."""
 
+from typing import Any
+
 from analysis.summarise import VERDICT_INSTRUCTION, prompt_input
 
 
-def _payload(*fdrs: float) -> dict[str, object]:
+def _payload(*fdrs: float) -> dict[str, Any]:
     return {
         "summary": {"type": "OVERREPRESENTATION"},
         "pathways_total": len(fdrs),
@@ -59,7 +61,7 @@ def test_a_pathway_exactly_on_the_threshold_counts_as_significant() -> None:
 def test_a_missing_fdr_is_not_significant() -> None:
     # Absence is normal in this API; it must never read as passing.
     payload = _payload(0.01)
-    del payload["pathways"][0]["entities"]["fdr"]  # type: ignore[index]
+    del payload["pathways"][0]["entities"]["fdr"]
     assert prompt_input(payload)["significant_among_shown"] == 0
 
 
@@ -121,3 +123,72 @@ def test_unmatched_identifiers_are_reported_as_a_count_not_a_proportion() -> Non
     assert "Never state a proportion" in UNMATCHED_INSTRUCTION
     # Nothing in the payload lets a proportion be computed.
     assert not any("submitted" in k or "total_identifiers" in k for k in out)
+
+
+def test_a_hit_resting_on_few_entities_is_flagged_fragile() -> None:
+    # US3 scenario 1, and the thing the spec says readers most often get
+    # wrong: "a pathway with 2 of 3 entities found is not strong evidence".
+    # Computed here because a model handed a tiny p-value and a tiny count
+    # describes the p-value.
+    payload = _payload(1e-9)
+    payload["pathways"][0]["entities"].update({"found": 2, "total": 3})
+    out = prompt_input(payload)
+    assert out["pathways"][0]["fragile"] is True
+    assert out["pathways"][0]["significant"] is True
+
+
+def test_a_well_supported_hit_is_not_flagged_fragile() -> None:
+    # A flag that is always true would pass the test above and say nothing.
+    payload = _payload(1e-9)
+    payload["pathways"][0]["entities"].update({"found": 40, "total": 120})
+    pathways: list[dict[str, Any]] = prompt_input(payload)["pathways"]
+    assert pathways[0]["fragile"] is False
+
+
+def test_the_fragility_threshold_is_on_the_count_not_the_ratio() -> None:
+    # 2 of 3 looks like a perfect hit by ratio and is nearly meaningless;
+    # 40 of 200 looks poor by ratio and is real evidence. The ratio is the
+    # misleading number here, so the flag deliberately ignores it.
+    ratio_perfect = _payload(1e-9)
+    ratio_perfect["pathways"][0]["entities"].update({"found": 2, "total": 3})
+    ratio_poor = _payload(1e-9)
+    ratio_poor["pathways"][0]["entities"].update({"found": 40, "total": 200})
+    pathways: list[dict[str, Any]] = prompt_input(ratio_perfect)["pathways"]
+    assert pathways[0]["fragile"] is True
+    poor: list[dict[str, Any]] = prompt_input(ratio_poor)["pathways"]
+    assert poor[0]["fragile"] is False
+
+
+def test_significant_before_correction_is_distinguishable_from_after() -> None:
+    # US3 scenario 2. Both numbers are present per pathway and `significant`
+    # is defined as after correction, so the two can be told apart rather
+    # than the model choosing which it means.
+    from analysis.summarise import STATISTICS_INSTRUCTION
+
+    payload = _payload(0.2)
+    payload["pathways"][0]["entities"]["pValue"] = 0.001
+    out = prompt_input(payload)
+    pathway = out["pathways"][0]
+    assert pathway["p_value"] == 0.001
+    assert pathway["fdr"] == 0.2
+    assert pathway["significant"] is False
+    assert "before or after" in STATISTICS_INSTRUCTION
+    assert "is after" in STATISTICS_INSTRUCTION
+
+
+def test_a_pathway_with_no_found_count_is_not_called_fragile() -> None:
+    # Absence is normal in this API and must not read as a finding either way.
+    payload = _payload(1e-9)
+    del payload["pathways"][0]["entities"]["found"]
+    pathways: list[dict[str, Any]] = prompt_input(payload)["pathways"]
+    assert pathways[0]["fragile"] is False
+
+
+def test_the_model_is_told_not_to_repeat_our_field_names() -> None:
+    # Measured against a real result: the summary said "these pathways are
+    # classified as fragile", handing the reader an internal label instead of
+    # the reason. The flag is ours; the explanation is theirs.
+    from analysis.summarise import STATISTICS_INSTRUCTION
+
+    assert "Never use the word 'fragile'" in STATISTICS_INSTRUCTION
+    assert "internal labels" in STATISTICS_INSTRUCTION