Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions specs/010-search-page-answers/research.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,46 @@ An endpoint that answers correctly and slowly, streaming, with verification. Not
against nothing, and is otherwise idle on this. p50 15.2s is too slow to ship to
users and entirely good enough to integrate against. The latency work is real and
separate, and spec 009 plus query-expansion reduction are its two known levers.

## Query expansion, measured 2026-09-20 (T020)

The task said "reduce query expansion from 5 variants". The premise was
wrong, and the measurement is the useful part.

| alternates | expansion call | retrieval | total | documents kept |
|---|---|---|---|---|
| 4 (default) | 1.27s | 1.22s | **2.49s** | baseline |
| 2 | 1.26s | 0.65s | 1.92s | 84% |
| 1 | 1.38s | 0.57s | 1.95s | 77% |
| 0 | 0.00s | 0.31s | **0.31s** | 71% |

**The expansion call costs about 1.27s whatever it returns.** Trimming four
variants to two saves fan-out only -- about 0.57s of a 2.49s stage. The whole
cost goes away only by not making the call, which is a different change from
the one the task described.

With expansion off, `answer-sweep` passed **13/13 in 79s**, against roughly
150s with it on. That is the largest single latency lever found so far, and
it bears on T022.

### Two things about how this was measured

The first attempt varied the count by editing the prompt to ask for "exactly
N". The model obeyed at 2 and **ignored 1 and 0, producing four either way**,
so those rows silently re-measured the baseline -- visible only because the
number of queries actually asked was recorded alongside the timings. The
count is now enforced in code, which removes the model's obedience from the
experiment and is also how the feature is implemented.

Document overlap is not quality. 71% of baseline documents at zero alternates
says three-quarters of the retrieved set is unchanged; it says nothing about
whether the quarter that changed mattered.

### Why the default is unchanged

Thirteen tracked questions establish that *those* answers do not need
expansion. They do not establish that recall is unaffected in general, and
expansion exists for the questions nobody wrote a test for. So this ships as
`QUERY_EXPANSION_ALTERNATES` with the default at 4 -- a switch and a
measurement, not a verdict. Trying 0 on beta, where the sweep and the routing
probe both run on every deploy, is the cheap way to learn more.
2 changes: 1 addition & 1 deletion specs/010-search-page-answers/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ 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
- [x] T020 Measure query expansion, and make the count configurable. **The task's premise was wrong**: the cost is the expansion *call*, not the number of variants it returns. Measured over six tracked questions -- 4 alternates 1.27s expand + 1.22s retrieve; 2 alternates 1.26s + 0.65s; 0 alternates 0.00s + 0.31s. Trimming the count saves only fan-out; the cost disappears only at zero, where `answer-sweep` was 13/13 in 79s against ~150s. `QUERY_EXPANSION_ALTERNATES` sets it and **the default is unchanged**: thirteen questions show those answers do not need expansion, not that recall is unaffected in general
- [ ] 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
- [ ] T021 Land spec 009 collection routing and re-measure. **Still open — I marked this done on 2026-09-18 and was wrong.** What landed is *source* routing (`resolve_active_sources` picks reactome / userguide / live). Collection routing is selecting among the five collections *within* the reactome bundle, and it is not implemented: `QueryIntent` has no `collections` field, `resolve_collections` does not exist, and `retrieve_documents` still loops over every collection
- [ ] T022 Re-assess FR-005 against the result and say plainly whether 2s/10s is reachable
Expand Down
81 changes: 75 additions & 6 deletions src/retrievers/csv_chroma.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import asyncio
import csv
import os
from collections.abc import Coroutine, Iterable
from contextvars import ContextVar
from pathlib import Path
Expand Down Expand Up @@ -54,6 +55,52 @@ def chroma_settings() -> chromadb.config.Settings:
return chromadb.config.Settings(anonymized_telemetry=False)


ALTERNATES_ENV = "QUERY_EXPANSION_ALTERNATES"
DEFAULT_ALTERNATES = 4


def expansion_alternates() -> int:
"""How many alternate questions to retrieve for, besides the original.

Measured 2026-09-20 over six tracked questions (spec 010, T020):

| alternates | expand | retrieve | total | documents kept |
|---|---|---|---|---|
| 4 (default) | 1.27s | 1.22s | 2.49s | baseline |
| 2 | 1.26s | 0.65s | 1.92s | 84% |
| 1 | 1.38s | 0.57s | 1.95s | 77% |
| 0 | 0.00s | 0.31s | 0.31s | 71% |

**The expansion call costs about 1.27s whatever it returns**, so trimming
the count saves only fan-out; the cost goes away only at zero. With zero,
`answer-sweep` was 13/13 and ran in 79s against about 150s.

The default is unchanged regardless. Thirteen questions establish that the
answers we track do not need expansion; they do not establish that recall
is unaffected in general, and expansion exists for the questions nobody
wrote a test for. This is a switch and a measurement, not a verdict.

An unparseable or negative value falls back to the default loudly, rather
than silently disabling a recall mechanism.
"""
raw = os.getenv(ALTERNATES_ENV, "").strip()
if not raw:
return DEFAULT_ALTERNATES
try:
value = int(raw)
except ValueError:
logger.warning(
"%s=%r is not a number; using %d", ALTERNATES_ENV, raw, DEFAULT_ALTERNATES
)
return DEFAULT_ALTERNATES
if value < 0:
logger.warning(
"%s=%d is negative; using %d", ALTERNATES_ENV, value, DEFAULT_ALTERNATES
)
return DEFAULT_ALTERNATES
return value


multi_query_prompt = PromptTemplate(
input_variables=["question"],
template="""You are a biomedical question expansion engine for information retrieval over the Reactome biological pathway database.
Expand Down Expand Up @@ -418,9 +465,12 @@ def _get_relevant_documents(
is appended AFTER the generated ones: RRF breaks ties by first
appearance, so reordering here would silently change the ranking.
"""
queries = self.query_expander.invoke(
{"question": query}, config={"callbacks": run_manager.get_child()}
)
wanted = expansion_alternates()
queries: list[str] = []
if wanted:
queries = self.query_expander.invoke(
{"question": query}, config={"callbacks": run_manager.get_child()}
)[:wanted]
if self.include_original:
queries.append(query)
return unique_documents(self.retrieve_documents(queries, run_manager))
Expand All @@ -429,12 +479,31 @@ async def _aget_relevant_documents(
self, query: str, *, run_manager: AsyncCallbackManagerForRetrieverRun
) -> list[Document]:
"""Async twin of the above; must agree with it document for document."""
queries = await self.query_expander.ainvoke(
{"question": query}, config={"callbacks": run_manager.get_child()}
queries = await self._expand(
query,
lambda: self.query_expander.ainvoke(
{"question": query}, config={"callbacks": run_manager.get_child()}
),
)
return unique_documents(await self.aretrieve_documents(queries, run_manager))

async def _expand(self, query: str, call: Any) -> list[str]:
"""The queries to retrieve for, honouring the configured count.

At zero the expansion call is skipped entirely rather than made and
discarded -- that call is the larger half of the cost, and making it
anyway would keep the expense while losing the benefit.

The original is appended LAST, as it always was: RRF breaks ties by
first appearance, so reordering silently changes the ranking.
"""
wanted = expansion_alternates()
queries: list[str] = []
if wanted:
queries = (await call())[:wanted]
if self.include_original:
queries.append(query)
return unique_documents(await self.aretrieve_documents(queries, run_manager))
return queries

def weighted_reciprocal_rank(
self, doc_lists: list[list[Document]]
Expand Down
101 changes: 101 additions & 0 deletions tests/retrievers/test_query_expansion.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
"""How many alternate questions retrieval asks for.

Measured, not argued (spec 010 T020): the expansion call costs about 1.27s
whatever it returns, so trimming the count saves only fan-out and the cost
disappears only at zero. The default is unchanged regardless -- thirteen
tracked questions show those answers do not need expansion, not that recall
is unaffected in general.
"""

import asyncio

import pytest

from retrievers.csv_chroma import (
ALTERNATES_ENV,
DEFAULT_ALTERNATES,
HybridRetriever,
expansion_alternates,
)


def test_the_default_is_unchanged_without_configuration(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.delenv(ALTERNATES_ENV, raising=False)
assert expansion_alternates() == DEFAULT_ALTERNATES == 4


@pytest.mark.parametrize(("raw", "expected"), [("0", 0), ("2", 2), ("7", 7)])
def test_a_configured_count_is_honoured(
monkeypatch: pytest.MonkeyPatch, raw: str, expected: int
) -> None:
monkeypatch.setenv(ALTERNATES_ENV, raw)
assert expansion_alternates() == expected


@pytest.mark.parametrize("raw", ["", " ", "lots", "-1", "3.5"])
def test_a_bad_value_falls_back_loudly_rather_than_disabling_recall(
monkeypatch: pytest.MonkeyPatch, raw: str, caplog: pytest.LogCaptureFixture
) -> None:
# The dangerous direction: a typo must not silently turn expansion off,
# because nothing downstream would look any different.
monkeypatch.setenv(ALTERNATES_ENV, raw)
assert expansion_alternates() == DEFAULT_ALTERNATES


class _Expander:
def __init__(self) -> None:
self.calls = 0

async def ainvoke(self, _inputs: object, config: object = None) -> list[str]:
self.calls += 1
return [f"alt {i}" for i in range(4)]


def _expand(
count: str | None, monkeypatch: pytest.MonkeyPatch
) -> tuple[list[str], int]:
if count is None:
monkeypatch.delenv(ALTERNATES_ENV, raising=False)
else:
monkeypatch.setenv(ALTERNATES_ENV, count)
retriever = HybridRetriever.__new__(HybridRetriever)
object.__setattr__(retriever, "include_original", True)
expander = _Expander()
queries = asyncio.run(
retriever._expand("the question", lambda: expander.ainvoke(None))
)
return queries, expander.calls


def test_the_original_question_is_always_asked_and_always_last(
monkeypatch: pytest.MonkeyPatch,
) -> None:
# RRF breaks ties by first appearance, so the position is ranking, not
# cosmetics.
for count in (None, "2", "0"):
queries, _ = _expand(count, monkeypatch)
assert queries[-1] == "the question"


def test_at_zero_the_expansion_call_is_skipped_not_made_and_discarded(
monkeypatch: pytest.MonkeyPatch,
) -> None:
# The call is the larger half of the cost. Making it and throwing the
# result away would keep the expense and lose the benefit -- and would
# look identical in every other measurement.
queries, calls = _expand("0", monkeypatch)
assert queries == ["the question"]
assert calls == 0


def test_a_lower_count_truncates_rather_than_trusting_the_prompt(
monkeypatch: pytest.MonkeyPatch,
) -> None:
# Measured: asked for "exactly 1" and "exactly 0" alternates, the model
# produced four anyway. Enforcing the count in code removes the model's
# obedience from the question.
queries, calls = _expand("2", monkeypatch)
assert queries == ["alt 0", "alt 1", "the question"]
assert calls == 1
Loading