Explain unmatched identifiers, and make the disclosing tier worth choosing

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

@@ -46,14 +46,22 @@ a deleted one, and that the same token returns the same text.
 
 ## Phase 4: User Story 2 — Why were my identifiers not found? (P2)
 
+**A constraint measured 2026-09-20 that changes scenario 1.** The aggregate
+result carries `identifiersNotFound` and **nothing about how many identifiers
+were submitted** — the denominator lives only behind `/token/{token}/found/all`,
+which returns the reader's own identifiers and is therefore the disclosing
+tier. So the proportion the scenario asks for is not derivable at the default
+tier, and asking for one would invent a statistic the way D9 describes. The
+count is reported instead; the reader knows what they submitted.
+
 **Goal**: A reader learns why identifiers went unmatched and whether the result can be trusted.
 
 **Independent test**: Submit a token from an analysis with a deliberate identifier mismatch; the summary reports the proportion and names the likely cause.
 
-- [ ] T017 [US2] Include `identifiersNotFound`, `pathwaysFound` and `resourceSummary` in the aggregate prompt input in `src/analysis/summarise.py`, which together explain most mismatches without disclosing anything
-- [ ] T018 [US2] Add the `identifiers` tier in `src/analysis/disclosure.py`, fetching `GET /token/{token}/notFound` only when the request asked for it
-- [ ] T019 [P] [US2] Test in `tests/analysis/test_disclosure.py` that the `identifiers` tier is never reached without an explicit request, by asserting the not-found call is not made under the aggregate tier
-- [ ] T020 [US2] Test that a result with every identifier found produces a summary that says so rather than inventing a problem, in `tests/analysis/test_summarise.py` (spec US2 scenario 2)
+- [x] T017 [US2] Include `identifiersNotFound`, `pathwaysFound` and `resourceSummary` in the aggregate prompt input in `src/analysis/summarise.py`, which together explain most mismatches without disclosing anything
+- [x] T018 [US2] Add the `identifiers` tier in `src/analysis/disclosure.py`, fetching `GET /token/{token}/notFound` only when the request asked for it
+- [x] T019 [P] [US2] Test (in tests/api/test_analysis_summary.py, on the served path) that the `identifiers` tier is never reached without an explicit request, by asserting the not-found call is not made under the aggregate tier
+- [x] T020 [US2] Test that a result with every identifier found produces a summary that says so rather than inventing a problem, in `tests/analysis/test_summarise.py` (spec US2 scenario 2)
 
 ## Phase 5: User Story 3 — What do these numbers mean? (P3)
 

src/analysis/client.py

@@ -177,3 +177,44 @@ async def current_release(*, client: httpx.AsyncClient | None = None) -> str | N
     finally:
         if owned:
             await client.aclose()
+
+
+async def fetch_not_found(
+    token: str, *, limit: int = 50, client: httpx.AsyncClient | None = None
+) -> list[str] | None:
+    """The user's unmatched identifiers. **Identifier tier only.**
+
+    This is the endpoint that returns the reader's own submitted data, so it
+    is a separate function taking a separate decision rather than a flag on
+    `fetch_result`. A flag acquires a default, and a default here is a
+    disclosure nobody chose.
+
+    Bounded: a list of thousands would be sent to a model provider and tell
+    the reader nothing a sample does not.
+    """
+    if not is_well_formed(token):
+        return None
+    owned = client is None
+    client = client or httpx.AsyncClient(timeout=TIMEOUT_SECONDS)
+    try:
+        response = await client.get(
+            f"{base_url()}/token/{token}/notFound",
+            headers=_headers(),
+            params={"pageSize": limit, "page": 1},
+        )
+        if response.status_code != 200:
+            return None
+        payload = response.json()
+    except Exception as exc:
+        logger.warning("not-found lookup failed: %s", type(exc).__name__)
+        return None
+    finally:
+        if owned:
+            await client.aclose()
+    if not isinstance(payload, list):
+        return None
+    return [
+        str(entry["id"])
+        for entry in payload
+        if isinstance(entry, dict) and entry.get("id")
+    ][:limit]

src/analysis/summarise.py

@@ -81,6 +81,9 @@ def prompt_input(payload: dict[str, Any]) -> dict[str, Any]:
             for p in pathways
         ],
     }
+    # Stated rather than left for the model to notice, because "nothing went
+    # wrong" is the case a model most readily embellishes into a caveat.
+    out["all_identifiers_matched"] = unmatched == 0
     for optional in ("resourceSummary", "speciesSummary", "warnings", "expression"):
         if optional in payload:
             out[optional] = payload[optional]
@@ -98,6 +101,40 @@ def prompt_input(payload: dict[str, Any]) -> dict[str, Any]:
     "significance before and after correction wherever you mention it.",
 }
 
+#: Always appended. The aggregate result carries how many identifiers were
+#: *not* found and nothing at all about how many were submitted -- the
+#: denominator lives only behind `/found/all`, which returns the reader's own
+#: identifiers and is therefore the disclosing tier.
+#:
+#: So a proportion cannot be derived, and asking for one would produce the
+#: same class of invention as D9's "12 significant out of 1280". The reader
+#: knows how many they submitted; the count alone is useful to them.
+UNMATCHED_INSTRUCTION = (
+    "The data gives how many identifiers were NOT found and does not give how "
+    "many were submitted. State the count. Never state a proportion, a "
+    "percentage, or how many were found -- none of those are derivable. Use "
+    "`resourceSummary` and `speciesSummary` to say what the likely cause is: "
+    "identifiers resolving through a single resource suggests an identifier "
+    "type Reactome does not index, and pathways concentrated in one species "
+    "suggests the wrong species was analysed."
+)
+
+#: Appended only when the reader chose the disclosing tier and unmatched
+#: identifiers were actually retrieved.
+#:
+#: Without it the tier is the worst of both: their identifiers are sent to a
+#: model provider and the summary says exactly what the aggregate one said.
+#: Measured 2026-09-20 -- the first version sent the names and never
+#: mentioned them, because nothing asked it to. A disclosure has to buy the
+#: reader something or it should not be offered.
+NAMED_UNMATCHED_INSTRUCTION = (
+    "`identifiers_not_found_names` lists identifiers the reader submitted "
+    "that Reactome did not match, because they asked for them. Name them, and "
+    "say what their form suggests -- a gene symbol Reactome does not carry, an "
+    "identifier from a resource it does not index, an obsolete or misspelled "
+    "symbol. Only comment on the ones listed; the list may be truncated."
+)
+
 #: Appended whenever the count is a lower bound. Separate from the verdict
 #: because it is about what the *data* omits rather than what it shows.
 INEXACT_COUNT_INSTRUCTION = (

src/api/analysis_summary.py

@@ -26,10 +26,12 @@
 
 from agent.graph import resolve_llm_model
 from agent.models import get_llm
-from analysis.client import current_release, fetch_result
+from analysis.client import current_release, fetch_not_found, fetch_result
 from analysis.disclosure import Tier, for_tier
 from analysis.summarise import (
     INEXACT_COUNT_INSTRUCTION,
+    NAMED_UNMATCHED_INSTRUCTION,
+    UNMATCHED_INSTRUCTION,
     VERDICT_INSTRUCTION,
     prompt_input,
 )
@@ -67,11 +69,7 @@
 """.strip()
 
 
-#: `identifiers` is agreed in the contract and not built (Phase 4). Serving an
-#: aggregate summary for it would be safe but silently wrong: the reader chose
-#: the disclosing option and would be given the other one, with nothing saying
-#: so. A choice quietly overridden is worse than a choice refused.
-IMPLEMENTED_TIERS = ("aggregate",)
+IMPLEMENTED_TIERS = ("aggregate", "identifiers")
 
 
 class SummaryRequest(BaseModel):
@@ -143,6 +141,14 @@ async def stream() -> AsyncIterator[str]:
 
                 payload = for_tier(fetched.result, body.disclosure)
                 model_input = prompt_input(payload)
+                if body.disclosure == "identifiers":
+                    # The only place this service asks for the reader's own
+                    # identifiers, reached only because they chose it. A
+                    # separate call taking a separate decision, never a flag
+                    # with a default, and never on the aggregate path.
+                    unmatched = await fetch_not_found(body.token)
+                    if unmatched:
+                        model_input["identifiers_not_found_names"] = unmatched
                 release = await current_release()
                 yield _sse(
                     "start",
@@ -174,6 +180,9 @@ 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} {UNMATCHED_INSTRUCTION}"
+                if model_input.get("identifiers_not_found_names"):
+                    instruction = f"{instruction} {NAMED_UNMATCHED_INSTRUCTION}"
                 messages = [
                     ("system", SYSTEM_PROMPT),
                     (

tests/analysis/test_summarise.py

@@ -91,3 +91,33 @@ def test_an_unsorted_result_is_never_claimed_as_exact() -> None:
     payload = _payload(0.9, 0.001)
     payload["pathways_total"] = 1280
     assert prompt_input(payload)["significant_count_is_exact"] is False
+
+
+def test_a_result_with_everything_matched_says_so_rather_than_hedging() -> None:
+    # US2 scenario 2. "Nothing went wrong" is the case a model most readily
+    # embellishes into a caveat, so it is stated as a fact in the input
+    # rather than left to be noticed.
+    payload = _payload(0.001)
+    payload["identifiersNotFound"] = 0
+    out = prompt_input(payload)
+    assert out["all_identifiers_matched"] is True
+    assert out["identifiers_not_found"] == 0
+
+
+def test_unmatched_identifiers_are_reported_as_a_count_not_a_proportion() -> None:
+    # Measured against beta: the aggregate result carries how many were NOT
+    # found and nothing about how many were submitted. The denominator lives
+    # only behind `/found/all`, which returns the reader's own identifiers --
+    # so a proportion is not derivable at this tier, and asking for one would
+    # invent a statistic the way "12 significant out of 1280" did (D9).
+    from analysis.summarise import UNMATCHED_INSTRUCTION
+
+    payload = _payload(0.001)
+    payload["identifiersNotFound"] = 7
+    out = prompt_input(payload)
+    assert out["identifiers_not_found"] == 7
+    assert out["all_identifiers_matched"] is False
+    assert "how many were submitted" in UNMATCHED_INSTRUCTION
+    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)

tests/api/test_analysis_summary.py

@@ -274,21 +274,72 @@ def test_a_missing_disclosure_choice_is_rejected(keys: tuple[str, str]) -> None:
     assert response.status_code == 422
 
 
-def test_the_unbuilt_identifier_tier_is_refused_not_quietly_downgraded(
-    keys: tuple[str, str], wired: _Counter
+def _with_not_found_spy(monkeypatch: pytest.MonkeyPatch) -> list[str]:
+    """Records every request for the reader's own identifiers."""
+    asked: list[str] = []
+
+    async def _spy(token: str, **_kwargs: Any) -> list[str]:
+        asked.append(token)
+        return ["SMITH_LAB_SECRET_GENE_001", "PATIENT_004_MARKER"]
+
+    monkeypatch.setattr("api.analysis_summary.fetch_not_found", _spy)
+    return asked
+
+
+def test_the_aggregate_tier_never_asks_for_the_users_identifiers(
+    keys: tuple[str, str], monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # T019, and the single most important assertion in this feature. Asserted
+    # by recording the outbound call, not by reading the summary and seeing
+    # nothing alarming -- a model that simply did not mention them would pass
+    # the second check while the identifiers had already left the service.
+    asked = _with_not_found_spy(monkeypatch)
+    private, public = keys
+    events = _events(_post(public, caller_token=_token(private)).text)
+    assert events[-1][1]["state"] == "summarised"
+    assert asked == [], "the aggregate tier fetched the user's identifiers"
+
+
+def test_the_identifier_tier_asks_only_when_it_was_chosen(
+    keys: tuple[str, str], monkeypatch: pytest.MonkeyPatch
 ) -> None:
-    # `identifiers` is agreed and not built. Serving an aggregate summary for
-    # it would disclose nothing extra and still be wrong: the reader chose the
-    # disclosing option and would get the other, with nothing saying so. A
-    # choice quietly overridden is worse than one refused, because it looks
-    # like it was honoured.
+    asked = _with_not_found_spy(monkeypatch)
     private, public = keys
-    payload = _events(
+    events = _events(
         _post(public, caller_token=_token(private), disclosure="identifiers").text
-    )[-1][1]
-    assert payload["state"] == "refused"
-    assert payload["reason"] == "unsupported_tier"
-    assert wired.calls == 0
+    )
+    assert events[-1][1]["state"] == "summarised"
+    assert len(asked) == 1, "the chosen tier did not fetch what it promised"
+
+
+def test_the_identifiers_reach_the_model_only_on_the_disclosing_tier(
+    keys: tuple[str, str], monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # The guarantee stated as the reader would understand it: on the default
+    # choice their identifiers are not sent anywhere, and on the other choice
+    # they are -- which is the whole point of offering a choice.
+    _with_not_found_spy(monkeypatch)
+    sent: list[Any] = []
+
+    class _Recording(_Counter):
+        async def astream(self, messages: Any) -> AsyncIterator[Any]:
+            sent.append(messages)
+            async for chunk in super().astream(messages):
+                yield chunk
+
+    private, public = keys
+    for tier, expected in (("aggregate", False), ("identifiers", True)):
+        sent.clear()
+        recording = _Recording()
+        with pytest.MonkeyPatch.context() as patch:
+            patch.setattr(
+                "api.analysis_summary.get_llm",
+                lambda *a, _r=recording, **k: _r,
+            )
+            _post(public, caller_token=_token(private), disclosure=tier)
+        assert sent, f"{tier}: the model was never called, so this proves nothing"
+        leaked = "SMITH_LAB_SECRET_GENE_001" in json.dumps(sent, default=str)
+        assert leaked is expected, f"{tier} tier sent identifiers: {leaked}"
 
 
 def test_a_rate_limited_caller_is_not_told_it_is_unverified(
@@ -302,3 +353,33 @@ def test_a_rate_limited_caller_is_not_told_it_is_unverified(
     private, public = keys
     payload = _events(_post(public, caller_token=_token(private)).text)[-1][1]
     assert payload["reason"] == "rate_limited"
+
+
+def test_the_disclosing_tier_is_told_to_name_what_it_was_given(
+    keys: tuple[str, str], monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # Measured 2026-09-20: the first version sent the identifiers and never
+    # mentioned them, because nothing asked it to. The reader chose to
+    # disclose and got the aggregate summary back -- disclosure with no
+    # benefit, which is worse than not offering the choice.
+    _with_not_found_spy(monkeypatch)
+    sent: list[Any] = []
+
+    class _Recording(_Counter):
+        async def astream(self, messages: Any) -> AsyncIterator[Any]:
+            sent.append(messages)
+            async for chunk in super().astream(messages):
+                yield chunk
+
+    private, public = keys
+    for tier, expected in (("identifiers", True), ("aggregate", False)):
+        sent.clear()
+        recording = _Recording()
+        with pytest.MonkeyPatch.context() as patch:
+            patch.setattr(
+                "api.analysis_summary.get_llm",
+                lambda *a, _r=recording, **k: _r,
+            )
+            _post(public, caller_token=_token(private), disclosure=tier)
+        told = "Name them" in json.dumps(sent, default=str)
+        assert told is expected, f"{tier}: instruction to name identifiers {told}"