From 1a40e396534b1e766e8a1b423f955822da6cba48 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 18 Sep 2026 05:36:11 +0000 Subject: [PATCH 1/2] Record why some answers have no citations, and the option to change it Found by measuring citation ordering for the website: a userguide question returns 0 citations and 452 tokens. Not a failure -- a `citation` carries a Reactome stable id, and the userguide collection indexes documentation pages whose metadata is a page URL, so there is nothing that can become one. Worth writing down because it is the opposite of what testing would suggest. It correlates with the *fastest* answers, so it is common rather than exotic, and the four questions anyone would naturally reach for all produce citations. Also records that citations always precede prose, and why that is structural rather than a timing accident: they are emitted on `on_retriever_end`, and token events are gated on the same event because retrieval completing is how the answer's tokens are separated from the query expander's. That makes "sources are complete when the first token arrives" a design fact a caller can rely on. The option to cite userguide pages is recorded in the shape the website proposed and I agreed -- an optional `url` sibling, never a fabricated `R-` id, which would resolve to nothing or to the wrong entity. Not scheduled, and Adam's call. Co-Authored-By: Claude Opus 5 --- .../contracts/answer_endpoint.md | 42 +++++++++++++++++++ specs/010-search-page-answers/tasks.md | 1 + 2 files changed, 43 insertions(+) diff --git a/specs/010-search-page-answers/contracts/answer_endpoint.md b/specs/010-search-page-answers/contracts/answer_endpoint.md index 34093ee..8735bc7 100644 --- a/specs/010-search-page-answers/contracts/answer_endpoint.md +++ b/specs/010-search-page-answers/contracts/answer_endpoint.md @@ -75,6 +75,48 @@ At most **12** citations are sent. Measured against the live endpoint this cap i binding on ordinary questions, so treat it as the most relevant few rather than the complete set. +### Some answers have no citations at all, by construction + +A `citation` carries `st_id`, a Reactome stable identifier, and only the Reactome +and disease-variant collections have them. The userguide collection indexes +documentation pages whose metadata is a page URL, so a userguide-routed question +produces **zero citations** -- measured 2026-09-18: "How do I use the pathway +browser?" returned 0 citations and 452 tokens. + +That is a good answer with no sources, not a failure. Two things follow for a +caller: + +- Render nothing rather than an empty heading. The website's panel keeps its + heading and list inside one conditional for this reason. +- It correlates with the **fastest** answers -- the userguide question is also + the 3.3s-to-first-token one -- so it is common rather than exotic, and it is + precisely what the four questions anyone would naturally test with fail to + produce. + +**Citations always precede prose**, and that is structural rather than +incidental: citations are emitted only on `on_retriever_end`, and token events +are gated on that same event, because retrieval completing is how the answer's +tokens are told apart from the query expander's. So sources are complete at the +moment the first token arrives, and a panel may render them before any prose. + +#### Open: citing userguide pages (not scheduled) + +Proposed by the website session 2026-09-18 and agreed in shape, not scheduled. +A documentation URL does not fit `{st_id, display_name}`, and **a fabricated +`R-` identifier is not an option** -- it would resolve to nothing, or worse to +the wrong entity. The shape that works is an optional sibling, exactly one of the +two present: + +``` +{"st_id": "R-HSA-8862803", "display_name": "..."} # unchanged +{"url": "/userguide/pathway-browser", "display_name": "..."} # new +``` + +A consumer that understands only `st_id` keeps working by skipping what it does +not recognise. Worth doing when this contract is next opened; an answer that is +honest about having no sources is much better than an invented one, so this is +not a blocker. + ### What `token` text contains **Markdown, never HTML.** Headings and lists appear; anchors do not. The answer diff --git a/specs/010-search-page-answers/tasks.md b/specs/010-search-page-answers/tasks.md index 835abd7..367206b 100644 --- a/specs/010-search-page-answers/tasks.md +++ b/specs/010-search-page-answers/tasks.md @@ -47,6 +47,7 @@ state. - [x] T021 [US2] Enforce `aud` on the caller token (asked for by the website, D1); the code refused every token carrying one, since PyJWT rejects `aud` when no audience is expected (PR #240) - [x] T022 Rename human_token -> caller_token everywhere; D1 established the token asserts caller identity, not humanity (PR #240) - [x] T023 Answer the website's cancellation question: a client hang-up raises CancelledError inside the answer generator and produces nothing further, so they need not cancel upstream (PR #240) +- [ ] T020b Decide whether userguide answers cite their pages. Needs an optional `url` sibling to `st_id` in the citation event -- agreed in shape with the website, not scheduled. A fabricated stable id is not an option - [ ] T020a Decide what to do about non-reproducible retrieval: three runs of one question shared only 4 of 19 citations (Jaccard 0.26) because query expansion is itself a model call. Affects what FR-007 can cache - [x] T018 [US2] Return `state: failed` with no partial answer on any internal error, so the page renders no panel (FR-006) - [x] T011a [US1] Strip inline HTML anchors from the token stream in src/util/anchor_strip.py; the contract promises prose without them and the chat prompt emits them, split across ~20 fragments (PR #236) From 0f8d2888a8b068f39558ff39a02f4f4ac549ea65 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 18 Sep 2026 05:42:30 +0000 Subject: [PATCH 2/2] Fix the task IDs I duplicated, and mark what is actually done Four IDs were duplicated, all by me: I added T021, T022, T023 and T020b across three PRs today without checking that Phase 5 already used them. Mine are renumbered rather than Phase 5's, because the older ones are referenced by the dependency notes. The list had also drifted from the code in both directions. Marked done after verifying each rather than from memory: the branch and baseline (T001, T002), the shared graph via lifespan (T004), the streaming surface (T005) and its more-than-one-token test (T006), spec 009 routing (T021 -- landed and live, and the main reason first-token fell from the ~36s once recorded to 9.6s), telling the website (T023), and recording the measured times (T024). T003 named tests/api/test_middleware_scope.py, which was never written. The behaviour it asked for is tested, in tests/util/test_captcha_scope.py, because the decision moved into a module that can be tested without an app. Recorded where it actually lives rather than left looking undone. T020b's "~16s of preprocessing" is corrected. It never reproduced, it was 5.1s before the two-round change and is 2.6s after, and the sequential half of that question is answered. What remains is whether a search-page question needs all four calls at all. 32 done, 5 open, and the five are genuinely open. Co-Authored-By: Claude Opus 5 --- specs/010-search-page-answers/tasks.md | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/specs/010-search-page-answers/tasks.md b/specs/010-search-page-answers/tasks.md index 367206b..687c9a3 100644 --- a/specs/010-search-page-answers/tasks.md +++ b/specs/010-search-page-answers/tasks.md @@ -11,15 +11,15 @@ website repo entirely. Speed is Phase 5 and does not gate the handover. ## Phase 1: Setup -- [ ] T001 Create branch `010-search-page-answers`; confirm `.specify/feature.json` points at specs/010-search-page-answers -- [ ] T002 [P] Record today's baseline in specs/010-search-page-answers/quickstart.md: p50 15.2s, p90 22.4s over the 15 sweep questions, so Phase 5 has a before +- [x] T001 Create branch `010-search-page-answers`; confirm `.specify/feature.json` points at specs/010-search-page-answers +- [x] T002 [P] Record today's baseline in specs/010-search-page-answers/quickstart.md: p50 15.2s, p90 22.4s over the 15 sweep questions, so Phase 5 has a before ## Phase 2: Foundational (blocks the endpoint) -- [ ] T003 Pin the captcha middleware's current behaviour in tests/api/test_middleware_scope.py: every path under CHAINLIT_URI redirects to the captcha page except the existing allowlist -- [ ] T004 Give `AgentGraph` one home the FastAPI app can reach, in bin/chat-fastapi.py via lifespan, so the endpoint and Chainlit share one instance (SC-003) rather than paying 51.5s of startup twice -- [ ] T005 Add a streaming surface to src/agent/graph.py using the compiled graph's `astream_events`, yielding answer tokens and retrieved documents -- [ ] T006 Test that T005 yields more than one token for a real question — not that it returns 200. A surface that completes without streaming is the failure this must catch +- [x] T003 Pin the captcha middleware's current behaviour — done in tests/util/test_captcha_scope.py (6 tests) rather than the tests/api/test_middleware_scope.py named here, because the decision moved into src/util/captcha_scope.py where it can be tested without an app +- [x] T004 Give `AgentGraph` one home the FastAPI app can reach, in bin/chat-fastapi.py via lifespan, so the endpoint and Chainlit share one instance (SC-003) rather than paying 51.5s of startup twice +- [x] T005 Add a streaming surface to src/agent/graph.py using the compiled graph's `astream_events`, yielding answer tokens and retrieved documents +- [x] T006 Test that T005 yields more than one token for a real question — not that it returns 200. A surface that completes without streaming is the failure this must catch ## Phase 3: User Story 1 — a verified person sees an answer forming (P1) @@ -44,10 +44,10 @@ state. - [x] T016 [US2] Test that no model call happens for a refused request in tests/api/test_answer_endpoint.py, by asserting on a patched graph rather than on timing (SC-002) - [x] T017 [P] [US2] Rate limit per token as a backstop in src/util/rate_limit.py; the budget is the website's, enforced before the call reaches here (FR-008). 30 per 10 minutes, keyed on `sub`/`jti` when D1 provides one and a token hash until then (PR #237) - [x] T017a [US2] Stop paying for a discarded web search: the endpoint took `enable_postprocess` at its default, so every answer ran a Tavily search that `astream_answer` has no event to return (PR #237) -- [x] T021 [US2] Enforce `aud` on the caller token (asked for by the website, D1); the code refused every token carrying one, since PyJWT rejects `aud` when no audience is expected (PR #240) -- [x] T022 Rename human_token -> caller_token everywhere; D1 established the token asserts caller identity, not humanity (PR #240) -- [x] T023 Answer the website's cancellation question: a client hang-up raises CancelledError inside the answer generator and produces nothing further, so they need not cancel upstream (PR #240) -- [ ] T020b Decide whether userguide answers cite their pages. Needs an optional `url` sibling to `st_id` in the citation event -- agreed in shape with the website, not scheduled. A fabricated stable id is not an option +- [x] T025 [US2] Enforce `aud` on the caller token (asked for by the website, D1); the code refused every token carrying one, since PyJWT rejects `aud` when no audience is expected (PR #240) +- [x] T026 Rename human_token -> caller_token everywhere; D1 established the token asserts caller identity, not humanity (PR #240) +- [x] T027 Answer the website's cancellation question: a client hang-up raises CancelledError inside the answer generator and produces nothing further, so they need not cancel upstream (PR #240) +- [ ] T028 Decide whether userguide answers cite their pages. Needs an optional `url` sibling to `st_id` in the citation event -- agreed in shape with the website, not scheduled. A fabricated stable id is not an option - [ ] T020a Decide what to do about non-reproducible retrieval: three runs of one question shared only 4 of 19 citations (Jaccard 0.26) because query expansion is itself a model call. Affects what FR-007 can cache - [x] T018 [US2] Return `state: failed` with no partial answer on any internal error, so the page renders no panel (FR-006) - [x] T011a [US1] Strip inline HTML anchors from the token stream in src/util/anchor_strip.py; the contract promises prose without them and the chat prompt emits them, split across ~20 fragments (PR #236) @@ -59,14 +59,14 @@ state. - [x] T019 Measure first-token and completion separately across the tracked questions; publish the distribution, not one question. Measured 2026-09-18, two runs each: first token p50 9.6s / p90 12.2s (n=26), completion p50 10.4s / p90 18.1s (n=30). The earlier "36.1s to first token" came from one question and does not reproduce (PR #238) - [x] T019a Run preprocessing in two rounds instead of four sequential calls in src/agent/profiles/react_to_me.py; the base class already overlapped, and this override discarded it (PR #238) - [ ] T020 Reduce query expansion from 5 variants, measuring recall with bin/retrieval_baseline — its own call plus a 5x retrieval fan-out -- [ ] T020b Establish whether the four preprocessing calls must be sequential, and whether a search-page question needs all of them. They cost ~16s before retrieval starts and produce 36 tokens between them — the largest block in front of the first answer token -- [ ] T021 Land spec 009 collection routing and re-measure +- [ ] T020c Establish whether a search-page question needs all four preprocessing calls. The sequential half of this is answered and done (T019a): they run in two rounds and cost 2.6s at the median, not the ~16s recorded here, which never reproduced +- [x] T021 Land spec 009 collection routing and re-measure — landed and live (`state["active_sources"][0]` selects at retrieval time), and re-measured 2026-09-18: it is the main reason first-token fell from the ~36s once recorded to 9.6s - [ ] T022 Re-assess FR-005 against the result and say plainly whether 2s/10s is reachable ## Phase 6: Handover -- [ ] T023 Tell the website session the endpoint exists, with a curl that streams, and what it does not yet do -- [ ] T024 [P] Update specs/010-search-page-answers/spec.md status and record the measured first-token and completion times +- [x] T023 Tell the website session the endpoint exists, with a curl that streams, and what it does not yet do — done 2026-09-18, including the nginx user-agent 403 and the measured latency; they have built against it and a token they minted verified +- [x] T024 [P] Update specs/010-search-page-answers/spec.md status and record the measured first-token and completion times — first token p50 9.6s / p90 12.2s, completion p50 10.4s / p90 18.1s ## Dependencies