diff --git a/specs/010-search-page-answers/contracts/answer_endpoint.md b/specs/010-search-page-answers/contracts/answer_endpoint.md index a893a40b..85e0c466 100644 --- a/specs/010-search-page-answers/contracts/answer_endpoint.md +++ b/specs/010-search-page-answers/contracts/answer_endpoint.md @@ -149,6 +149,28 @@ linked phrase in the middle. Where the model used a link as a trailing citation, that leaves the pathway title as a bare clause -- cosmetic, and the structured `citation` events are the reliable source for links. +**No trailing source list.** The answer prompts ask for a bullet list of every +citation at the end, which the chat UI renders. This caller gets its links from +the `citation` events, so that list is a duplicate -- and after the anchors come +off, a duplicate with no links in it. The endpoint drops the heading and +everything after it. + +Callers **must not** pattern-match the heading themselves. Until 2026-09-19 the +prompts specified no heading at all, only "a bullet-point list of each unique +citation anchor", so the model invented one per answer: `## Sources`, +`## Most relevant sources`, `relevant references`, `Key sources`, +`Top citations`. The website was matching those and could not win, because it +was fitting samples from an unconstrained generator. The prompts now pin the +heading to exactly: + +``` +## Sources +``` + +and `SourcesSectionStripper` removes it on the served path. The stripper still +accepts the older variants, because a prompt is an instruction and not a +guarantee, but no caller needs to know that. + ## Properties worth holding to **It must be safe to ignore.** Any failure, timeout, refusal or unverified caller diff --git a/src/api/answer.py b/src/api/answer.py index af50801f..74c6c576 100644 --- a/src/api/answer.py +++ b/src/api/answer.py @@ -33,6 +33,7 @@ from util.caller_token import TokenRejectedError, verify from util.logging import logging from util.rate_limit import identity_of, limiter_from_env +from util.sources_section import SourcesSectionStripper logger = logging.getLogger(__name__) @@ -123,6 +124,10 @@ async def stream() -> AsyncIterator[str]: # separate events, so they come out here -- across fragment boundaries, # because one anchor arrives as twenty-odd fragments. stripper = AnchorStripper() + # And the trailing source list goes too: this caller renders citations + # from the `citation` events, so the prose copy is a duplicate -- and a + # worse one, since `AnchorStripper` has just taken its links off. + sources = SourcesSectionStripper() tokens_sent = 0 try: async with asyncio.timeout(ANSWER_TIMEOUT_SECONDS): @@ -139,7 +144,7 @@ async def stream() -> AsyncIterator[str]: enable_postprocess=False, ): if event.kind == "token": - text = stripper.feed(event.text) + text = sources.feed(stripper.feed(event.text)) if text: tokens_sent += 1 yield _sse("token", {"text": text}) @@ -202,7 +207,7 @@ async def stream() -> AsyncIterator[str]: # terminal event to stop waiting. logger.exception("answering %r failed", body.question[:80]) state = "failed" - held = stripper.flush() + held = sources.feed(stripper.flush()) + sources.flush() if held: yield _sse("token", {"text": held}) yield _sse( diff --git a/src/retrievers/plantreactome/prompt.py b/src/retrievers/plantreactome/prompt.py index 763195dd..8426b50c 100644 --- a/src/retrievers/plantreactome/prompt.py +++ b/src/retrievers/plantreactome/prompt.py @@ -25,6 +25,10 @@ - Use accessible language while maintaining technical precision. - Ensure the narrative flows logically, presenting background, mechanisms, and significance 5. Source list at the end: After the main narrative, provide a bullet-point list of each unique citation anchor exactly once, in the same Node Name format. + - Head the list with exactly this line and nothing else: `## Sources` + Not a variation on it. The search page strips this section by that + exact heading, because it renders the citations itself; a different + wording leaves the reader a duplicate list. - Examples: - Mitosis - Cell Cycle diff --git a/src/retrievers/reactome/prompt.py b/src/retrievers/reactome/prompt.py index aaf5689a..b6afa3ca 100644 --- a/src/retrievers/reactome/prompt.py +++ b/src/retrievers/reactome/prompt.py @@ -28,6 +28,10 @@ - Use accessible language while maintaining technical precision. - Ensure the narrative flows logically, presenting background, mechanisms, and significance 5. Source list at the end: After the main narrative, provide a bullet-point list of each unique citation anchor exactly once, in the same Node Name format. + - Head the list with exactly this line and nothing else: `## Sources` + Not a variation on it. The search page strips this section by that + exact heading, because it renders the citations itself; a different + wording leaves the reader a duplicate list. - Examples: - Apoptosis - Cell Cycle diff --git a/src/retrievers/userguide/prompt.py b/src/retrievers/userguide/prompt.py index 046298cb..8b4d3b92 100644 --- a/src/retrievers/userguide/prompt.py +++ b/src/retrievers/userguide/prompt.py @@ -23,6 +23,10 @@ - Use accessible language; avoid unnecessary jargon. - Prefer numbered steps for multi-step procedures. 5. Source list at the end: After the main answer, provide a bullet-point list of each unique citation anchor exactly once, in the same display_name format. + - Head the list with exactly this line and nothing else: `## Sources` + Not a variation on it. The search page strips this section by that + exact heading, because it renders the citations itself; a different + wording leaves the reader a duplicate list. - Examples: - Pathway Browser - Searching Reactome diff --git a/src/util/sources_section.py b/src/util/sources_section.py new file mode 100644 index 00000000..02d4df82 --- /dev/null +++ b/src/util/sources_section.py @@ -0,0 +1,148 @@ +"""Drop the trailing source list from a token stream split at arbitrary points. + +The search page renders citations as its own chips, built from the `citation` +events, so the prose list at the end of an answer is a strictly worse copy of +data the caller already has -- worse still after `AnchorStripper`, which leaves +it as bare display names with no links. + +The website was matching the heading itself and could not win. Measured +2026-09-19, our prompts never specified one: they asked for "a bullet-point +list of each unique citation anchor" and said nothing about a heading, so the +model invented one per answer -- `## Sources`, `## Most relevant sources`, +`relevant references`, `Key sources`, `Top citations`. That is not five +phrasings of a contract, it is five samples from an unconstrained generator. + +The prompts now pin the heading to `## Sources`. This is the second half: strip +it here so no caller has to pattern-match model output at all. The pattern below +is still tolerant, because the prompt is an instruction and not a guarantee, but +it is bounded -- a heading or bold-only line, at most five words, naming sources +-- rather than any line mentioning the word. +""" + +import re + +# A whole line that is a markdown heading or a bold-only line naming nothing +# but the source list. Two rules keep it off real content, and the first was +# learned the hard way: an earlier version allowed any short heading that +# mentioned sources, and "## Sources of reactive oxygen species" -- an entirely +# plausible Reactome heading -- truncated the answer there. +# +# 1. The noun must be the LAST word. "Sources of oxidative stress" is about +# biology; "Most relevant sources" is a source list. +# 2. Only a closed set of qualifiers may precede it, so "Cellular sources" is +# left alone. +# +# The asymmetry justifies the strictness. A heading this misses costs the +# reader a duplicate list at the end -- cosmetic, and exactly what the website +# lives with today. A heading this matches wrongly costs them the rest of the +# answer. When in doubt, do not match. +_QUALIFIER = ( + r"(?:most|more|relevant|key|top|main|primary|all|further|additional" + r"|related|supporting|complete|full|cited|used)" +) +_HEADING = re.compile( + r"^[ \t]*(?:#{1,6}[ \t]+|\*\*[ \t]*)" + rf"(?:{_QUALIFIER}[ \t]+){{0,3}}" + r"(?:sources?|references?|citations?)" + r"[ \t]*:?[ \t]*(?:\*\*)?[ \t]*:?[ \t]*$", + re.IGNORECASE | re.MULTILINE, +) + +# Held back while a partial line could still turn out to be that heading. A +# heading is one short line; beyond this the text is prose and is released. +_MAX_HELD = 120 + + +def _could_become_heading(partial: str) -> bool: + """True while an unterminated line might still turn into the heading. + + A heading starts a line with `#` or `**`. A bullet (`* `) cannot, and + neither can prose, so both stream straight through. + """ + stripped = partial.lstrip(" \t") + if len(stripped) > _MAX_HELD: + return False # Too long to be a heading; it is prose. + if not stripped: + return True + if stripped[0] == "#": + return True + return stripped == "*" or stripped.startswith("**") + + +class SourcesSectionStripper: + """Feed fragments in, get the answer without its trailing source list. + + Everything from the heading to the end of the stream is dropped: the + prompts place the list last, so there is nothing after it to keep. + """ + + def __init__(self) -> None: + self._buffer = "" + self._done = False + # Whether the buffer currently begins at a real start of line. Once + # text has been released, position 0 is mid-line -- and `^` under + # re.MULTILINE matches there anyway, which turned a bolded word in + # mid-sentence into a heading and ate the rest of the answer. + self._at_line_start = True + + def feed(self, text: str) -> str: + if self._done: + return "" + self._buffer += text + # Terminated only: mid-stream, "## Sources" matches before + # " of reactive oxygen species" has arrived, and deciding then drops + # the rest of a perfectly good answer. Only the whole-string probe + # caught this; the character-by-character one is what found it. + match = self._find_heading(terminated_only=True) + if match: + out = self._buffer[: match.start()] + self._buffer = "" + self._done = True + return out + # Hold back only a partial line that could still become that heading. + # Holding every unterminated line instead would defeat the point of + # the endpoint: an answer often has no newline until it ends, so the + # whole thing would arrive in one blob at `flush`. Two existing tests + # caught exactly that. + newline = self._buffer.rfind("\n") + if newline == -1 and not self._at_line_start: + cut = len(self._buffer) # No line start in here at all. + else: + cut = newline + 1 + if not _could_become_heading(self._buffer[cut:]): + cut = len(self._buffer) + out, self._buffer = self._buffer[:cut], self._buffer[cut:] + if out: + self._at_line_start = out.endswith("\n") + return out + + def flush(self) -> str: + """Whatever is still held, once the stream has ended.""" + if self._done: + return "" + match = self._find_heading(terminated_only=False) + held = self._buffer[: match.start()] if match else self._buffer + self._buffer = "" + self._done = True + return held + + def _find_heading(self, *, terminated_only: bool) -> re.Match[str] | None: + """The heading, ignoring a match at offset 0 when that is mid-line. + + `terminated_only` rejects a match that runs to the end of the buffer, + because more of that line may still arrive. At `flush` the stream has + ended, so there is nothing more to wait for. + """ + position = 0 + while True: + match = _HEADING.search(self._buffer, position) + if match is None: + return None + if terminated_only and match.end() >= len(self._buffer): + return None # The line has not ended; it may yet grow. + if match.start() or self._at_line_start: + return match + newline = self._buffer.find("\n") + if newline == -1: + return None + position = newline + 1 diff --git a/tests/api/test_answer_endpoint.py b/tests/api/test_answer_endpoint.py index 5270b081..d82fa5b4 100644 --- a/tests/api/test_answer_endpoint.py +++ b/tests/api/test_answer_endpoint.py @@ -488,3 +488,44 @@ async def drive() -> None: assert any( "abandoned by the caller" in message for message in messages ), f"no record of the abandoned stream; logged: {messages}" + + +def test_the_trailing_source_list_never_reaches_the_caller( + keys: tuple[str, str], monkeypatch: pytest.MonkeyPatch +) -> None: + # The website renders citations as chips from the `citation` events, so the + # prose list at the end is a duplicate -- and after AnchorStripper has taken + # the links off, a worse one. They were matching the heading and could not + # win: our prompts never specified one, so the model invented a different + # heading per answer. Stripped here instead, on the served path. + stub = _StubGraph( + [ + AnswerEvent(kind="citation", st_id="R-HSA-1", display_name="Apoptosis"), + AnswerEvent(kind="token", text="CDK5 phosphorylates tau.\n\n"), + AnswerEvent(kind="token", text="## Most rele"), + AnswerEvent(kind="token", text="vant sources\n- "), + AnswerEvent( + kind="token", + text='Apoptosis\n', + ), + AnswerEvent(kind="done", state="answered"), + ] + ) + monkeypatch.setattr("api.answer.get_graph", lambda *_a, **_k: stub) + private_pem, public_pem = keys + response = _client(public_pem).post( + f"{PREFIX}/answer", + json={"question": "what does CDK5 do?", "caller_token": _token(private_pem)}, + ) + assert response.status_code == 200 + prose = "".join( + json.loads(data)["text"] + for kind, data in _events(response.text) + if kind == "token" + ) + assert prose.strip() == "CDK5 phosphorylates tau." + assert "sources" not in prose.lower() + assert "Apoptosis" not in prose, "the citation event is the list, not the prose" + # The citation itself must survive: stripping the prose copy must not cost + # the caller the data it renders chips from. + assert any(kind == "citation" for kind, _ in _events(response.text)) diff --git a/tests/util/test_sources_section.py b/tests/util/test_sources_section.py new file mode 100644 index 00000000..15d18a47 --- /dev/null +++ b/tests/util/test_sources_section.py @@ -0,0 +1,127 @@ +"""The trailing source list must go, and nothing else with it.""" + +import pytest + +from util.sources_section import SourcesSectionStripper + +ANSWER = ( + "CDK5 phosphorylates tau in Alzheimer disease, and the reaction is " + "curated in Reactome.\n\n" +) + +# Every heading below was actually observed by the website session before the +# prompts pinned one. They are the reason this exists. +OBSERVED = ( + "## Sources", + "### Sources", + "## Most relevant sources", + "**Most relevant sources**", + "## Key sources", + "## Top citations", + "## Relevant references", + "**Sources:**", +) + + +@pytest.mark.parametrize("heading", OBSERVED) +def test_an_observed_heading_and_everything_after_it_goes(heading: str) -> None: + stripper = SourcesSectionStripper() + out = stripper.feed(f"{ANSWER}{heading}\n- Apoptosis\n- Cell Cycle\n") + assert stripper.flush() == "" + assert out == ANSWER + + +@pytest.mark.parametrize("heading", OBSERVED) +def test_it_survives_being_split_at_every_character(heading: str) -> None: + # The heading arrives in fragments like any other text: measured on the + # live endpoint, one anchor came as twenty-odd pieces. + whole = f"{ANSWER}{heading}\n- Apoptosis\n" + stripper = SourcesSectionStripper() + out = "".join(stripper.feed(c) for c in whole) + stripper.flush() + assert out == ANSWER + + +def test_prose_mentioning_sources_is_not_a_heading() -> None: + # The failure that would matter: eating the answer. A sentence about + # sources is not a source list, and neither is a bolded phrase inside one. + prose = ( + "Reactome draws on several **sources** of evidence, and the primary " + "sources for this reaction are listed in the literature references. " + "Citations of this pathway are numerous.\n" + ) + stripper = SourcesSectionStripper() + assert "".join(stripper.feed(c) for c in prose) + stripper.flush() == prose + + +def test_a_long_bold_line_is_prose_not_a_heading() -> None: + # Bounded at five words: a bold sentence that happens to end in "sources" + # is prose, and dropping the rest of the answer would be the worst + # possible failure here. + text = ( + "**The following mechanism is supported by several independent " + "curated sources**\n\nIt proceeds in three steps.\n" + ) + stripper = SourcesSectionStripper() + assert "".join(stripper.feed(c) for c in text) + stripper.flush() == text + + +def test_an_answer_with_no_source_list_is_unchanged() -> None: + stripper = SourcesSectionStripper() + assert stripper.feed(ANSWER) + stripper.flush() == ANSWER + + +def test_nothing_is_emitted_after_the_heading_even_in_later_feeds() -> None: + stripper = SourcesSectionStripper() + stripper.feed(f"{ANSWER}## Sources\n") + assert stripper.feed("- Apoptosis\n") == "" + assert stripper.flush() == "" + + +def test_prose_is_not_held_back_waiting_for_a_newline() -> None: + # The first version held every unterminated line, so an answer with no + # newline until the end arrived as one blob at flush -- which defeats the + # endpoint. Two endpoint tests caught it; this states it directly. + stripper = SourcesSectionStripper() + assert stripper.feed("CDK5 ") == "CDK5 " + assert stripper.feed("phosphorylates ") == "phosphorylates " + assert stripper.feed("tau.") == "tau." + assert stripper.flush() == "" + + +def test_a_bullet_is_not_mistaken_for_the_start_of_a_heading() -> None: + # "* " is a bullet; only "**" can open the bold form of the heading. + stripper = SourcesSectionStripper() + assert stripper.feed("- one\n* two") == "- one\n* two" + + +# Headings a real Reactome answer could plausibly use. Every one of these +# truncated the answer in the first version of this stripper, which allowed any +# short heading mentioning sources. The bound was on length, not on meaning. +BIOLOGY = ( + "## Sources of reactive oxygen species", + "## Sources of oxidative stress", + "### Key sources of ROS in mitochondria", + "**Sources of variation**", + "## Cellular sources", + "## Literature references for this pathway", + "## Endogenous sources of DNA damage", +) + + +@pytest.mark.parametrize("heading", BIOLOGY) +def test_a_heading_about_biology_does_not_eat_the_answer(heading: str) -> None: + body = "\n\nMitochondrial complex I is the principal contributor.\n" + text = f"Opening line.\n{heading}{body}" + stripper = SourcesSectionStripper() + out = "".join(stripper.feed(c) for c in text) + stripper.flush() + assert out == text + + +def test_the_asymmetry_is_deliberate() -> None: + # A heading this misses costs the reader a duplicate list -- cosmetic, and + # what the website lives with today. A heading this matches wrongly costs + # the rest of the answer. So an unrecognised variant must pass through + # rather than be guessed at. + text = "Answer.\n\n## Bibliography\n- Apoptosis\n" + stripper = SourcesSectionStripper() + assert "".join(stripper.feed(c) for c in text) + stripper.flush() == text