From 65128c85499272cf6bdac8afc4d31924699524f4 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Thu, 10 Sep 2026 19:31:20 +0000 Subject: [PATCH] Answer in the user's language Spec 004, stages 1 and 2. detected_language reaches the answer prompt in React-to-Me and Plant Reactome, as its own variable. Reproduced before fixing: "Quel role joue TP53 dans l'apoptose ?" was detected as French, rephrased to English, retrieved well -- and answered in English. Four steps of five already worked; the fifth dropped the value. chat_history still held the original French question and the model answered in English anyway, so it was never that the model could not tell. The mechanism is the whole point. create_retrieval_chain passes only `input` to the retriever -- retrieval_docs = (lambda x: x["input"]) | retriever -- so a separate prompt variable cannot reach BM25, the vector store, or the query expansion in front of them. #140 appended the instruction to `input` instead; measured through the whole retriever, that changes about half the fused documents. A test asserts the retrieval query is byte-identical across four languages, which is exact where a retrieval baseline would only show noise. The instruction's wording is @bleedblack1's, kept because it gets the hard part right: gene symbols, protein names, pathway names, R-HSA ids and URLs must survive untranslated. SET, MAX and CAT are gene symbols and ordinary English words. An adversarial review of the plan caught it contradicting the spec. FR-007 promised English questions "no additional prompt content" while the plan always passed the language -- which does add a sentence to every English prompt. FR-007 was the wrong half: it now promises no extra model call and a byte-identical retrieval query, and states the added sentence plainly rather than hiding it. Branching instead would leave the common path exercised only by non-English users. Verified against the Release95 bundle. French question -> French answer, R-HSA ids intact, 40 documents. English question -> unchanged English answer, 40 documents. Reverting the call site fails the test that covers it. Worth watching: the French answer carried 2 R-HSA citations against the English answer's 9. One sample, not a finding, but the kind of thing an answer-quality run should look at. Co-Authored-By: Claude Opus 5 --- specs/004-answer-in-user-language/plan.md | 99 ++++++++++++++++++ specs/004-answer-in-user-language/spec.md | 13 ++- specs/004-answer-in-user-language/tasks.md | 52 +++++++++ src/agent/profiles/plantreactome.py | 4 + src/agent/profiles/react_to_me.py | 4 + src/agent/tasks/language_instruction.py | 26 +++++ src/retrievers/plantreactome/prompt.py | 3 + src/retrievers/reactome/prompt.py | 3 + tests/agent/test_answer_language.py | 116 +++++++++++++++++++++ 9 files changed, 317 insertions(+), 3 deletions(-) create mode 100644 specs/004-answer-in-user-language/plan.md create mode 100644 specs/004-answer-in-user-language/tasks.md create mode 100644 src/agent/tasks/language_instruction.py create mode 100644 tests/agent/test_answer_language.py diff --git a/specs/004-answer-in-user-language/plan.md b/specs/004-answer-in-user-language/plan.md new file mode 100644 index 0000000..93c67cb --- /dev/null +++ b/specs/004-answer-in-user-language/plan.md @@ -0,0 +1,99 @@ +# Implementation Plan: Answer in the User's Language + +**Branch**: `feat/answer-in-user-language` | **Date**: 2026-09-10 | **Spec**: [spec.md](./spec.md) + +## Summary + +Pass `detected_language` to the answer prompt as its own variable, in the two +profiles that ignore it. Nothing else moves. + +The constraint that decides the design: **`input` must stay exactly as it is**, +because `create_retrieval_chain` hands it to the retriever, and `HybridRetriever` +hands it to the query expander. Both must keep seeing the English rephrasing and +nothing else. That is what rules out #140's mechanism and what makes this change +provably free of retrieval risk — the retrieval query is byte-identical before and +after, which a test can assert directly rather than measure statistically. + +## Technical Context + +**Language/Version**: Python 3.12 + +**Primary Dependencies**: `langchain-core` prompts; no new dependency. + +**Testing**: pytest. `bin/retrieval_baseline` is *not* needed — see below. + +**Constraints**: English questions must take exactly today's path (FR-007), with no +added prompt content and no extra model call. + +**Scale/Scope**: Two prompt templates, two call sites, one shared instruction. + +## Constitution Check + +| Article | How this plan satisfies it | +|---|---| +| I — verify the user path | The exit criterion is asking the assembled chain a French question and reading the answer, which is how the bug was found. | +| II — measure, don't argue | Retrieval is unchanged **by construction**: `input` is not touched. A test asserts the retrieval query is byte-identical, which is stronger than a baseline diff and does not spend an hour of API time. | +| III — characterization tests pin behaviour | A test pins that an English question produces no language instruction at all. | +| IV — fail loudly | Nothing to fail: an absent language falls back to today's behaviour, which is correct English output. | +| V — derive from the source of truth | The language comes from `BaseState`, where the detector already put it. Nothing re-detects. | +| VI — bias to doing over filing | Two contributed PRs are resolved rather than left open. | + +**No violations.** + +## Implementation Stages + +### Stage 1 — React-to-Me answers in the detected language + +Add a language instruction to the reactome answer prompt as a template variable, and +pass `state["detected_language"]` at the call site in `generate_answer`. + +The instruction carries #140's nomenclature rule, which is the part of that PR worth +keeping: gene symbols, protein names, pathway names, `R-HSA-*` identifiers and URLs +stay in English. + +**What an English question pays.** No extra model call, and a byte-identical +retrieval query — but it does gain the instruction in its answer prompt, saying to +answer in English. + +An earlier draft of this plan claimed English "costs nothing", which contradicted the +spec's own FR-007 and was simply untrue: a sentence added to every prompt is a +change, however small. FR-007 has been corrected to say what is actually guaranteed. +The cost is one sentence against roughly 3,200 tokens of retrieved context; the +alternative is a branch whose common path only non-English users exercise. + +**Exit criteria**: a French question is answered in French with nomenclature intact; +the retrieval query is byte-identical to today's; an English question still gets a +well-formed English answer. + +### Stage 2 — Plant Reactome, identically + +The same change to `plantreactome/prompt.py` and its call site. It is a separate +stage only because it is a separate deployment and can be verified separately. + +**Exit criteria**: same three, against the plantreactome profile. + +### Stage 3 — Close #125 and #140 + +Close both with credit, saying specifically what each contributed: #140's target and +nomenclature rule, #125's mechanism. Say plainly why #140's mechanism was not taken, +with the measured number — half the retrieved context changes. + +#125's hallucination-grading and web-search work is **not** resolved by this and must +not be described as such; it belongs to #123. + +**Exit criteria**: both closed with credit; the spec records the outcome. + +## Complexity Tracking + +| Decision | Simpler alternative rejected | Why | +|---|---|---| +| A prompt variable | Append to `input`, as #140 does | Measured: about half the fused documents change. The instruction reaches the retriever and the query expander, neither of which should see it. | +| Instruct the generator | Translate the finished answer | D1. An extra model call and its latency on every non-English message, plus translation errors over a scientific answer, to gain a nomenclature protection the prompt already achieves. | +| Always pass the language | Branch on "is it English" | A branch means two paths, one of which is rarely exercised. Passing "English" is the same code doing the same thing. | +| No baseline run | `bin/retrieval_baseline` before and after | The retrieval query is unchanged by construction; a test asserting that is exact, where a baseline diff would only show noise and cost an hour. | + +## Out of Scope + +- Verifying the answer really is in the requested language. +- The corpus, embeddings, or the detector. +- #125's hallucination grader and web search — #123's decision. diff --git a/specs/004-answer-in-user-language/spec.md b/specs/004-answer-in-user-language/spec.md index 286a902..011fccd 100644 --- a/specs/004-answer-in-user-language/spec.md +++ b/specs/004-answer-in-user-language/spec.md @@ -236,8 +236,14 @@ the Reactome deployment is the larger audience. - **FR-005**: Scientific nomenclature — gene symbols, protein names, pathway names, `R-HSA-*` identifiers — MUST NOT be translated. - **FR-006**: URLs and citation links MUST NOT be translated or altered. -- **FR-007**: An English question MUST follow exactly the current path, with no - additional model call and no additional prompt content. +- **FR-007**: An English question MUST cost no additional model call, and MUST reach + the retriever with a byte-identical query. + + It does gain a short instruction in the *answer* prompt, saying to answer in + English. That is a real change for every current user and is stated rather than + hidden. The alternative — branching on "is this English" — creates two paths where + the common one is untested by anyone who only ever asks in English, and the + instruction is a sentence against roughly 3,200 tokens of retrieved context. - **FR-008**: The language MUST reach the answer prompt as its own input, not concatenated into another field. @@ -259,7 +265,8 @@ the Reactome deployment is the larger audience. what the same question produces today. - **SC-004**: In a non-English answer, every gene symbol, pathway name and `R-HSA-*` identifier appears in English, and every Reactome URL resolves. -- **SC-005**: An English question costs the same number of model calls as today. +- **SC-005**: An English question costs the same number of model calls as today, and + its answer remains in English and well-formed. ## Decisions for the team diff --git a/specs/004-answer-in-user-language/tasks.md b/specs/004-answer-in-user-language/tasks.md new file mode 100644 index 0000000..014947d --- /dev/null +++ b/specs/004-answer-in-user-language/tasks.md @@ -0,0 +1,52 @@ +--- +description: "Task list for answering in the user's language" +--- + +# Tasks: Answer in the User's Language + +**Input**: [plan.md](./plan.md), [spec.md](./spec.md) + +**Tests**: Included. FR-003 and FR-007 are both "nothing changed" claims, and an +unasserted claim of that kind is worth nothing. + +## Phase 1: Foundational + +- [x] T001 Add a shared language instruction constant carrying the nomenclature rule (gene symbols, protein names, pathway names, `R-HSA-*`, URLs stay English) in `src/agent/tasks/language_instruction.py`, crediting @bleedblack1 for the wording + +## Phase 2: User Story 1 — React-to-Me answers in the detected language (P1) + +**Independent test**: ask the assembled chain a French question; read the answer. + +- [x] T002 [US1] Add a `{detected_language}` variable and the instruction to `src/retrievers/reactome/prompt.py` +- [x] T003 [US1] Pass `state["detected_language"]` in `generate_answer` in `src/agent/profiles/react_to_me.py`, leaving `input` untouched +- [x] T004 [P] [US1] Test in `tests/agent/test_answer_language.py`: the retrieval query is byte-identical with and without a language (FR-003) +- [x] T005 [P] [US1] Test in `tests/agent/test_answer_language.py`: the prompt receives the language as its own variable, never concatenated into `input` (FR-008) +- [x] T006 [P] [US1] Test in `tests/agent/test_answer_language.py`: an English question adds no extra model call and reaches the retriever with a byte-identical query (FR-007) +- [x] T006a [US1] Ask the real chain an English question and confirm the answer is still English and well-formed — the instruction is new prompt content for every existing user (SC-005) +- [x] T007 [US1] Ask the real chain a French question against the Release95 bundle; confirm the answer is French and gene symbols, `R-HSA-*` IDs and URLs are unchanged (Article I) + +## Phase 3: User Story 3 — Plant Reactome, identically (P2) + +- [x] T008 [US3] Same change to `src/retrievers/plantreactome/prompt.py` and its call site in `src/agent/profiles/plantreactome.py` +- [x] T009 [P] [US3] Test that both profiles use the same shared instruction, so they cannot drift + +## Phase 4: Polish & Cross-Cutting + +- [x] T010 Perturbation check: revert the call site and confirm the French test fails +- [x] T011 Run `ruff check`, `ruff format --check`, `mypy`, `pytest` +- [ ] T012 Close #140 with credit to @bleedblack1 — the target and the nomenclature rule are kept; the mechanism is not, with the measured number +- [ ] T013 Close #125 with credit to @bhavyakeerthi3 for the mechanism, noting its hallucination-grading work belongs to #123 and is untouched +- [ ] T014 Record the outcome in `specs/004-answer-in-user-language/spec.md` + +## Dependencies + +```text +T001 -> Phase 2 (T002-T007) -> Phase 3 (T008-T009) -> Phase 4 +``` + +Phase 3 depends on Phase 2 only for the shared constant's final shape. + +## Implementation Strategy + +**MVP is Phase 2.** React-to-Me is the deployment with users. Phase 3 is the same +change to a second profile and could ship separately. diff --git a/src/agent/profiles/plantreactome.py b/src/agent/profiles/plantreactome.py index 368b337..8ebe7be 100644 --- a/src/agent/profiles/plantreactome.py +++ b/src/agent/profiles/plantreactome.py @@ -80,6 +80,10 @@ async def call_model( result: dict[str, Any] = await self.plantreactome_rag.ainvoke( { "input": state["rephrased_input"], + # A separate variable, never concatenated into `input`: + # create_retrieval_chain passes `input` alone to the retriever, so + # anything folded into it reaches BM25 and the query expander. + "detected_language": state["detected_language"], "chat_history": ( state["chat_history"] if state["chat_history"] diff --git a/src/agent/profiles/react_to_me.py b/src/agent/profiles/react_to_me.py index bad9ef8..345663f 100644 --- a/src/agent/profiles/react_to_me.py +++ b/src/agent/profiles/react_to_me.py @@ -158,6 +158,10 @@ async def generate_answer( result: dict[str, Any] = await rag.ainvoke( { "input": state["rephrased_input"], + # A separate variable, never concatenated into `input`: + # create_retrieval_chain passes `input` alone to the retriever, so + # anything folded into it reaches BM25 and the query expander. + "detected_language": state["detected_language"], "chat_history": ( state["chat_history"] if state["chat_history"] diff --git a/src/agent/tasks/language_instruction.py b/src/agent/tasks/language_instruction.py new file mode 100644 index 0000000..38a6ec9 --- /dev/null +++ b/src/agent/tasks/language_instruction.py @@ -0,0 +1,26 @@ +"""The instruction that makes an answer come back in the user's language. + +Shared by every answer prompt so the two deployments cannot drift apart. + +The wording is @bleedblack1's from #140, which got the hard part right: the +retrieved context is English and must stay English, but scientific nomenclature +inside the answer must not be translated either. `SET`, `MAX` and `CAT` are gene +symbols and also ordinary English words, and `R-HSA-9612973` means nothing +translated. + +What is not taken from #140 is where it put this. That PR appends it to `input`, +which `create_retrieval_chain` hands straight to the retriever -- verified in +`langchain_classic`: `retrieval_docs = (lambda x: x["input"]) | retriever`. Measured +through the whole retriever, that changes about half the fused documents. Here it is +a separate prompt variable, so `input` is untouched and retrieval, including the +query expansion in front of it, is byte-identical. +""" + +LANGUAGE_INSTRUCTION = """Answer in {detected_language}. + +The context you were given is in English because the Reactome knowledgebase is +English-only. Your answer must still be in {detected_language}. + +Keep gene symbols, protein names, pathway names, Reactome identifiers (R-HSA-...) +and URLs exactly as they appear in the context. Do not translate them, even when a +symbol is also an ordinary word.""" diff --git a/src/retrievers/plantreactome/prompt.py b/src/retrievers/plantreactome/prompt.py index 50fcd1f..786d510 100644 --- a/src/retrievers/plantreactome/prompt.py +++ b/src/retrievers/plantreactome/prompt.py @@ -1,5 +1,7 @@ from langchain_core.prompts import ChatPromptTemplate, MessagesPlaceholder +from agent.tasks.language_instruction import LANGUAGE_INSTRUCTION + plantreactome_system_prompt = """ You are an expert in molecular biology with access to the **Plant Reactome Knowledgebase**. Your primary responsibility is to answer the user's questions **comprehensively, mechanistically, and with precision**, drawing strictly from the **Plant Reactome Knowledgebase**. @@ -35,6 +37,7 @@ [ ("system", plantreactome_system_prompt), MessagesPlaceholder(variable_name="chat_history"), + ("system", LANGUAGE_INSTRUCTION), ("user", "Context:\n{context}\n\nQuestion: {input}"), ] ) diff --git a/src/retrievers/reactome/prompt.py b/src/retrievers/reactome/prompt.py index d570cb9..d348939 100644 --- a/src/retrievers/reactome/prompt.py +++ b/src/retrievers/reactome/prompt.py @@ -1,5 +1,7 @@ from langchain_core.prompts import ChatPromptTemplate, MessagesPlaceholder +from agent.tasks.language_instruction import LANGUAGE_INSTRUCTION + reactome_system_prompt = """ You are an expert in molecular biology with access to the **Reactome Knowledgebase**. Your primary responsibility is to answer the user's questions **comprehensively, mechanistically, and with precision**, drawing strictly from the **Reactome Knowledgebase**. @@ -35,6 +37,7 @@ [ ("system", reactome_system_prompt), MessagesPlaceholder(variable_name="chat_history"), + ("system", LANGUAGE_INSTRUCTION), ("user", "Context:\n{context}\n\nQuestion: {input}"), ] ) diff --git a/tests/agent/test_answer_language.py b/tests/agent/test_answer_language.py new file mode 100644 index 0000000..d838ed6 --- /dev/null +++ b/tests/agent/test_answer_language.py @@ -0,0 +1,116 @@ +"""The language reaches the answer prompt and nothing else. + +The whole design rests on one property: `create_retrieval_chain` passes only +`input` to the retriever -- + + retrieval_docs = (lambda x: x["input"]) | retriever + +-- so a language instruction placed anywhere else cannot affect what is retrieved. +#140 placed it inside `input`, and measured through the whole retriever that changed +about half the fused documents. These tests pin the property that makes this +approach different. +""" + +from pathlib import Path +from typing import Any + +import pytest + +pytest.importorskip("langchain", reason="retrieval stack not installed") + +from langchain_core.documents import Document # noqa: E402 +from langchain_core.retrievers import BaseRetriever # noqa: E402 + +from agent.tasks.language_instruction import LANGUAGE_INSTRUCTION # noqa: E402 + +REPO_ROOT = Path(__file__).parent.parent.parent + + +class _RecordingRetriever(BaseRetriever): + """Captures exactly what the chain asks it to retrieve.""" + + seen: list[str] + + def _get_relevant_documents(self, query: str, **kwargs: Any) -> list[Document]: + self.seen.append(query) + return [Document(page_content="TP53 induces apoptosis via BAX.")] + + +def _retrieval_query(**extra: Any) -> str: + """Build the real chain over a recording retriever and return the query it saw.""" + from langchain_classic.chains.combine_documents import ( + create_stuff_documents_chain, + ) + from langchain_classic.chains.retrieval import create_retrieval_chain + from langchain_core.language_models.fake_chat_models import FakeListChatModel + + from retrievers.reactome.prompt import reactome_qa_prompt + + retriever = _RecordingRetriever(seen=[]) + chain = create_retrieval_chain( + retriever=retriever, + combine_docs_chain=create_stuff_documents_chain( + llm=FakeListChatModel(responses=["an answer"]), prompt=reactome_qa_prompt + ), + ) + chain.invoke({"input": "What role does TP53 play in apoptosis?", **extra}) + return retriever.seen[0] + + +def test_the_language_never_reaches_the_retriever() -> None: + """FR-003. The property the whole approach depends on.""" + with_language = _retrieval_query(detected_language="French", chat_history=[]) + + assert with_language == "What role does TP53 play in apoptosis?" + assert "French" not in with_language + assert "CRITICAL" not in with_language, "no instruction prose in the query" + + +def test_the_retrieval_query_is_identical_whatever_the_language() -> None: + """FR-007: byte-identical, not merely similar. + + Stronger than a retrieval baseline and free: if the query cannot differ, the + documents cannot differ, so there is nothing to measure statistically. + """ + queries = { + _retrieval_query(detected_language=lang, chat_history=[]) + for lang in ("English", "French", "Japanese", "German") + } + assert len(queries) == 1, f"the retrieval query varied by language: {queries}" + + +def test_the_instruction_lives_in_the_prompt_not_the_input() -> None: + """FR-008. The difference between this and #140, asserted structurally.""" + for name in ("react_to_me", "plantreactome"): + source = (REPO_ROOT / "src" / "agent" / "profiles" / f"{name}.py").read_text() + assert '"detected_language": state["detected_language"],' in source, name + assert ( + 'state["rephrased_input"],' in source + ), f"{name} must pass the rephrasing alone as input" + + +def test_the_instruction_protects_scientific_nomenclature() -> None: + """#140's contribution, and the reason a naive translation is wrong. + + SET, MAX and CAT are gene symbols and ordinary English words; R-HSA-9612973 + means nothing translated. + """ + text = LANGUAGE_INSTRUCTION.lower() + for term in ("gene symbols", "protein names", "pathway names", "r-hsa", "url"): + assert term in text, f"the instruction must protect {term}" + assert "do not translate" in text + + +def test_both_profiles_share_one_instruction() -> None: + """Two deployments, one wording -- they cannot drift apart. + + Matched on the instruction's distinctive phrase rather than on "R-HSA", which + appears legitimately in both prompts as a citation example. A cruder check + passed on nothing and failed on that. + """ + for name in ("reactome", "plantreactome"): + source = (REPO_ROOT / "src" / "retrievers" / name / "prompt.py").read_text() + assert "LANGUAGE_INSTRUCTION" in source, name + assert ( + "Do not translate them" not in source + ), f"{name} inlines its own copy of the instruction instead of sharing it"