diff --git a/specs/010-search-page-answers/contracts/answer_endpoint.md b/specs/010-search-page-answers/contracts/answer_endpoint.md index 85e0c46..dbcd3a6 100644 --- a/specs/010-search-page-answers/contracts/answer_endpoint.md +++ b/specs/010-search-page-answers/contracts/answer_endpoint.md @@ -64,6 +64,17 @@ data: {"state": "answered", "seconds": 8.4} `state` is one of `answered`, `nothing_found`, `refused`, `failed`. +> **This endpoint does not require a human-presence claim and +> `/api/analysis-summary` does.** A caller token without one gets a full +> answer here and `no_human` there. Deliberate: this returns public pathway +> text, that one returns a reader's own uploaded analysis. +> +> **`/api/analysis-summary` uses different names for the same ideas**: +> `summarised` for success and `not_found` for the empty case. The event +> shapes are nearly identical, so do not reuse one state list for both — a +> consumer did, and a correct summary rendered as a truncated failure. See +> `specs/011-summarise-analysis-results/contracts/summary_endpoint.md`. + ### Why citations are separate events So the website renders links in its own style. Returning prose with embedded HTML diff --git a/specs/011-summarise-analysis-results/contracts/summary_endpoint.md b/specs/011-summarise-analysis-results/contracts/summary_endpoint.md index d8f01a9..4f82a86 100644 --- a/specs/011-summarise-analysis-results/contracts/summary_endpoint.md +++ b/specs/011-summarise-analysis-results/contracts/summary_endpoint.md @@ -27,6 +27,21 @@ identifiers, filenames, sample names or expression column labels. ### Human presence +**This endpoint requires it and `/api/answer` does not.** A caller token with +no presence claim gets a full answer from `/api/answer` and `no_human` from +here. That asymmetry is deliberate and load-bearing — it is why the stricter +gate can be applied to one route and not the other — and it was not written +down until the website measured it on 2026-09-21. + +The reason is what each endpoint discloses. `/api/answer` returns public +pathway text, and the search path it serves has no human gate at all; spec +010's D1 settled that its token asserts *service identity* and deliberately +says nothing about a person. This endpoint sends a user's own uploaded +analysis to a model provider, and at the disclosing tier their submitted +identifiers with it. The choice of what to disclose is only meaningful if a +person made it. + + `caller_token` carries three additional claims, minted only when the website's Turnstile-backed identity cookie validated on that request: @@ -95,7 +110,20 @@ data: {"state": "summarised", "seconds": 6.2} ``` `state` is one of `summarised`, `not_found`, `gone`, `unsupported`, `refused`, -`failed`. Anything but `summarised` means render no summary. Always HTTP 200 — +`failed`. + +> **These are not `/api/answer`'s state names, and the two are easy to +> conflate.** That endpoint's success state is `answered` and its empty state +> is `nothing_found`; this one's are `summarised` and `not_found`. The event +> shapes are otherwise nearly identical, so a consumer building both panels +> from one mental model will map a *successful* summary onto an unrecognised +> state — which, if the fallback is `failed`, renders a complete and correct +> summary as a truncated failure above and below the text. That happened on +> 2026-09-21. +> +> `release` is a **number** on both, though it reaches this one as text from +> the Analysis Service and is parsed here. It is null when unparseable, which +> is a legitimate value on both endpoints. Anything but `summarised` means render no summary. Always HTTP 200 — never an error code, so the analysis page cannot be broken by this service. **No prose source list.** Citations arrive as `citation` events, as on diff --git a/src/api/analysis_summary.py b/src/api/analysis_summary.py index 662ceb9..e9d7eb5 100644 --- a/src/api/analysis_summary.py +++ b/src/api/analysis_summary.py @@ -91,6 +91,22 @@ class SummaryRequest(BaseModel): disclosure: Tier +def _as_number(release: str | None) -> int | None: + """The release as a number, or None if it is not one. + + Kept separate from the storage key, which stays the raw string: the key + only has to be stable, while the contract field has to match the other + endpoint's type. + """ + if release is None: + return None + try: + return int(release) + except ValueError: + logger.warning("release %r is not a number; reporting null", release) + return None + + def _sse(event: str, payload: dict[str, Any]) -> str: return f"event: {event}\ndata: {json.dumps(payload)}\n\n" @@ -206,7 +222,14 @@ async def stream() -> AsyncIterator[str]: yield _sse( "start", { - "release": release, + # A number, matching `/api/answer`'s `release`. The + # Analysis Service answers `/database/version` as + # text, so this arrives as a string and went out as + # one -- the same field in the same event shape with + # a different type on each endpoint. A consumer that + # required a number got null and did not notice, + # because null is a legitimate value here. + "release": _as_number(release), "analysis_type": model_input.get("analysis_type"), # Stability is reuse, not determinism (FR-015). This # is how the interface knows which it is looking at. diff --git a/tests/api/test_analysis_summary.py b/tests/api/test_analysis_summary.py index 54251f8..2f468c7 100644 --- a/tests/api/test_analysis_summary.py +++ b/tests/api/test_analysis_summary.py @@ -167,7 +167,7 @@ def test_a_verified_human_caller_gets_a_summary(keys: tuple[str, str]) -> None: assert "citation" in kinds assert "token" in kinds start = events[0][1] - assert start["release"] == "97" + assert start["release"] == 97 assert start["analysis_type"] == "OVERREPRESENTATION" assert start["cached"] is False assert events[-1][1]["state"] == "summarised" @@ -790,3 +790,32 @@ def test_the_specific_presence_failure_is_logged_but_never_returned( logged = " ".join(r.getMessage() for r in caplog.records) assert expected_log in logged, f"not diagnosable from the log: {logged}" assert expected_log not in response.text, "the detail reached the caller" + + +def test_release_is_a_number_as_the_answer_endpoint_sends_it( + keys: tuple[str, str], +) -> None: + # The Analysis Service answers `/database/version` as text, so this + # arrived as a string while `/api/answer` sends an int -- the same field + # in the same event shape with a different type on each endpoint. A + # consumer required a number, got null, and did not notice, because null + # is legitimate here. + private, public = keys + start = _events(_post(public, caller_token=_token(private)).text)[0][1] + assert start["release"] == 97 + assert isinstance(start["release"], int) + + +def test_a_non_numeric_release_is_null_rather_than_a_string( + keys: tuple[str, str], monkeypatch: pytest.MonkeyPatch +) -> None: + # Null is already a legitimate value for this field, so degrading to it + # keeps the type honest. Emitting the raw string would put a second type + # back on the wire for the case nobody tests. + async def _odd() -> str: + return "97-beta" + + monkeypatch.setattr("api.analysis_summary.current_release", _odd) + private, public = keys + start = _events(_post(public, caller_token=_token(private)).text)[0][1] + assert start["release"] is None diff --git a/tests/api/test_endpoint_contracts_agree.py b/tests/api/test_endpoint_contracts_agree.py new file mode 100644 index 0000000..6be713f --- /dev/null +++ b/tests/api/test_endpoint_contracts_agree.py @@ -0,0 +1,84 @@ +"""The two SSE endpoints, held against each other. + +Each endpoint's contract was correct about itself and they diverged anyway: +different success-state names, and `release` an int on one and a string on +the other. Nobody was wrong; nothing compared them. A consumer did, and +rendered a correct summary as a truncated failure. + +Prose in both contracts is what came out of that. Prose does not fail when +the next divergence appears, so these do. +""" + +from pathlib import Path + +from agent.graph import AnswerEvent +from api.analysis_summary import _as_number + +ANSWER_CONTRACT = Path("specs/010-search-page-answers/contracts/answer_endpoint.md") +SUMMARY_CONTRACT = Path( + "specs/011-summarise-analysis-results/contracts/summary_endpoint.md" +) + +#: What each endpoint's `done` can carry. The answer endpoint's come from the +#: graph's own Literal; the summary's are written here because they are +#: string literals in the handler with nothing to import. +ANSWER_STATES = {"answered", "nothing_found", "refused", "failed"} +SUMMARY_STATES = { + "summarised", + "not_found", + "gone", + "unsupported", + "refused", + "failed", +} + + +def test_the_answer_states_are_still_what_the_contract_says() -> None: + # Taken from the graph rather than restated, so a new state there fails + # here rather than silently escaping both contracts. + declared = set(AnswerEvent.__annotations__["state"].__args__[0].__args__) + assert declared == ANSWER_STATES + + +def test_the_two_endpoints_disagree_only_where_both_contracts_say_so() -> None: + # `refused` and `failed` mean the same thing on both. Everything else is + # deliberately different, and each contract has to warn about the other + # -- because the shapes are near-identical and a consumer will reuse one + # state list for both. One did. + shared = ANSWER_STATES & SUMMARY_STATES + assert shared == {"refused", "failed"} + + answer_doc = ANSWER_CONTRACT.read_text() + summary_doc = SUMMARY_CONTRACT.read_text() + for state in SUMMARY_STATES - shared: + assert state in summary_doc, f"{state} undocumented" + for state in ANSWER_STATES - shared: + assert state in answer_doc, f"{state} undocumented" + + # And each must name the other's divergent success state, which is the + # one that caused actual harm. + assert "summarised" in answer_doc, "the answer contract does not warn" + assert "answered" in summary_doc, "the summary contract does not warn" + + +def test_release_is_the_same_type_on_both_endpoints() -> None: + # It was an int on one and a string on the other, in the same field of + # the same event. A consumer requiring a number got null from one of + # them and did not notice, because null is legitimate on both. + import typing + + from util.embedding_environment import EmbeddingEnvironment + + # The answer endpoint puts `get_release`'s value straight on the wire, + # so its declared return type is the contract. A declaration check, not + # a behavioural one -- reading the real value needs an installed bundle, + # and it is checked against the deployed build in the quickstart. + declared = typing.get_type_hints(EmbeddingEnvironment.get_release)["return"] + assert declared == (int | None) + + # The summary endpoint's arrives as text from the Analysis Service, so + # the normalisation is where its type is decided. That half is checked + # by behaviour. + assert _as_number("97") == 97 + assert _as_number(None) is None + assert _as_number("97-beta") is None