Spec 002: choosing the default answering model; close out spec 001

specs/001-retriever-rewrite/plan.md

@@ -2,6 +2,10 @@
 
 **Branch**: `plan/retriever-rewrite` | **Date**: 2026-09-08 | **Spec**: [spec.md](./spec.md)
 
+**Status**: Executed. See [Outcome](./spec.md#outcome-2026-09-09) in the spec —
+two of the exit criteria below turned out to be unfalsifiable or wrong, and
+which ones is recorded there.
+
 **Input**: Feature specification from `/specs/001-retriever-rewrite/spec.md`
 
 ## Summary

specs/001-retriever-rewrite/spec.md

@@ -4,7 +4,7 @@
 
 **Created**: 2026-09-08
 
-**Status**: Decisions settled — D1 by Helia, D2–D4 recorded below. Ready for `/speckit-plan`.
+**Status**: **Complete.** All three stages shipped 2026-09-09 (#182, #183, #185). Outcome recorded at the end of this file, including two predictions that were wrong.
 
 **Input**: Rewrite the Reactome retriever: replace the `HybridRetriever` that subclasses `MultiQueryRetriever` with a plain `BaseRetriever`, decide whether `SelfQueryRetriever` is replaced by plain semantic search, and make the context budget caller-supplied.
 
@@ -230,3 +230,52 @@ calls.
 - BM25 stays. Helia's proposal keeps it, it is deterministic, and it costs no LLM call.
 - Answer quality is judged by the ragas work, not by this spec. This spec changes *what reaches the model*; whether that improves answers is measured separately.
 - The rewrite happens before the LangChain upgrade, so that a behaviour change and an upgrade change cannot be confused for one another.
+
+
+---
+
+## Outcome (2026-09-09)
+
+All three stages shipped. `HybridRetriever` is a plain `BaseRetriever`;
+`SelfQueryRetriever` is gone from the pipeline; the bundle and the context budget
+are constructor arguments. Retrieval makes **one LLM call per message, down from
+21**. The five reaches into LangChain internals are zero, so the upgrade is
+unblocked — which was the point.
+
+Suppressions deleted rather than annotated: four `retrievers.*.rag` mypy baseline
+entries and four `B008` noqa comments.
+
+| stage | PR | what landed |
+|---|---|---|
+| 1 | #182 | plain `BaseRetriever`, RRF vendored, byte-identical results |
+| 2 | #183 | plain semantic search replaces SelfQuery |
+| 3 | #185 | `require_dir` for the bundle, budget as a constructor argument |
+
+### Two predictions in the plan were wrong
+
+Recorded because the point of this file is that the next person does not repeat
+them.
+
+1. **"Stage 1 must show *zero* difference"** was unfalsifiable as written. Chroma's
+   ANN search has a noise floor — two identical runs already differed on 2 of 8
+   question-collections — so "zero" could never have been observed, and a real
+   regression of that size would have been invisible against it. An exit criterion
+   has to be stated against measured noise, not against an ideal.
+
+2. **"Stage 2 becomes deterministic"** did not happen. Removing the LLM from the
+   vector side removed *that* source of variance; the ANN variance underneath it
+   stayed. Removing one of two causes does not make an effect go away, and the plan
+   asserted it would without checking which cause dominated.
+
+### One thing this rewrite did not do
+
+It changed what reaches the model four times over — over-fetch, de-duplication,
+genuine BM25/vector fusion, and plain semantic search — and **none of those changes
+has been evaluated for answer quality**. The plan said so under Out of Scope, which
+made it a known gap rather than an oversight, but it is still a gap.
+
+`src/evaluation/evaluator.py` is the tool for it and cannot be used as it stands: it
+builds its own `SelfQueryRetriever` + `EnsembleRetriever` + `MergerRetriever` rather
+than the shipping pipeline, so after Stage 2 it measures a configuration that no
+longer exists. Carried into [spec 002](../002-default-llm-choice/spec.md) as its P1,
+where the same fix serves both.

specs/002-default-llm-choice/checklists/requirements.md

@@ -0,0 +1,50 @@
+# Specification Quality Checklist: Choosing the Default Answering Model
+
+**Purpose**: Validate specification completeness and quality before proceeding to planning
+**Created**: 2026-09-09
+**Feature**: [spec.md](../spec.md)
+
+## Content Quality
+
+- [x] No implementation details (languages, frameworks, APIs)
+- [x] Focused on user value and business needs
+- [x] Written for non-technical stakeholders
+- [x] All mandatory sections completed
+
+## Requirement Completeness
+
+- [x] No [NEEDS CLARIFICATION] markers remain
+- [x] Requirements are testable and unambiguous
+- [x] Success criteria are measurable
+- [x] Success criteria are technology-agnostic (no implementation details)
+- [x] All acceptance scenarios are defined
+- [x] Edge cases are identified
+- [x] Scope is clearly bounded
+- [x] Dependencies and assumptions identified
+
+## Feature Readiness
+
+- [x] All functional requirements have clear acceptance criteria
+- [x] User scenarios cover primary flows
+- [x] Feature meets measurable outcomes defined in Success Criteria
+- [x] No implementation details leak into specification
+
+## Notes
+
+Three deviations from the template, each deliberate:
+
+1. **Model and file names appear in the spec.** `gpt-5.6-luna`, `evaluator.py`,
+   `resolve_temperature`. The subject of this decision *is* a named third-party
+   model and a named drifted file; writing around them would make the document
+   unusable for the three people who have to act on it.
+
+2. **The evidence section leads.** The template puts user stories first. Here the
+   measurements are the substance and the stories follow from them — and two of
+   the measurements are explicitly labelled as too weak to decide on, which is the
+   most important thing on the page.
+
+3. **No [NEEDS CLARIFICATION] markers, but three open questions.** They are not
+   gaps in the specification — the spec is complete and actionable as written. They
+   are decisions reserved to the team, recorded in "Decisions for the team" so they
+   are answered once and in the open rather than assumed. The P1 work (FR-001..004)
+   proceeds regardless of how they are answered.

specs/002-default-llm-choice/spec.md

@@ -0,0 +1,275 @@
+# Feature Specification: Choosing the Default Answering Model
+
+**Feature Branch**: `spec/default-llm-choice`
+
+**Created**: 2026-09-09
+
+**Status**: Evidence gathered, decision open. Three questions for the team below.
+
+**Input**: `gpt-4o-mini` is today's default. `gpt-5.6-luna` is now usable (#186). Does it become the default, on which surfaces, and what evidence settles it?
+
+## Why this is a specification and not a dependency bump
+
+The constitution reserves Spec Kit for "design work with real, unmade decisions" and
+excludes "dependency bumps, where the ceremony costs more than the fix". Changing the
+answering model looks like a bump and is not one:
+
+- it changes **what every user reads**, on every surface, immediately
+- it trades **determinism** for capability — an unavoidable consequence, not a setting
+- it changes **latency** by roughly a factor of two on a chat interface
+- it changes **cost**, in a direction nobody has yet measured
+- and the same choice will be made again for chat-alongside-search and analysis
+  summarisation, which have different tolerances
+
+None of that is recoverable from the diff. That is what this file is for.
+
+## What has already been settled
+
+`gpt-5.6-luna` **works end to end**. #186 removed the two things that stopped it:
+
+| blocker | resolution |
+|---|---|
+| `temperature=0.0` hardcoded; the gpt-5.5/5.6/6 families accept only their default of 1 | `resolve_temperature()` per model family, `LLM_TEMPERATURE` to override |
+| the three graders used `function_calling`, which gpt-5.6 refuses on `/v1/chat/completions` | `method="json_schema"`, verified on **both** models so the path cannot rot |
+
+Switching is now one environment variable: `LLM_MODEL=gpt-5.6-luna`. **This
+specification is about whether to set it, not how.**
+
+## Evidence gathered
+
+Measured through `AgentGraph.ainvoke` on the React-to-Me profile — the entry point
+`bin/chat-chainlit.py` uses — against the Release95 reactome bundle.
+
+### Latency and shape (3 questions, averaged)
+
+| | seconds/question | LLM calls/question | input tokens |
+|---|---|---|---|
+| gpt-4o-mini | 22.5 | 6 | ~2685 |
+| gpt-5.6-luna | 41.2 | 6 | ~3238 |
+
+Output tokens are omitted deliberately: the OpenAI callback undercounts streamed
+completions, and inconsistently between the two models, so the numbers it gave
+would have looked authoritative and been wrong.
+
+### Determinism (10 runs each)
+
+`temperature=1` is not a preference; it is the only value these models accept. The
+concern is the graders, which gate the whole conversation.
+
+| input | gpt-4o-mini @ 0.0 | gpt-5.6-luna @ 1.0 |
+|---|---|---|
+| science question → intent | `reactome` ×10 | `reactome` ×10 |
+| how-to question → intent | `userguide` ×10 | `userguide` ×10 |
+| benign text → safety | `true` ×10 | `true` ×10 |
+| prompt injection → safety | `false` ×10 | `false` ×10 |
+
+Stable on four inputs. That is evidence, not a guarantee — four inputs is a smoke
+test, and the failure mode being ruled out is a rare flip, which is precisely what
+a small sample cannot rule out.
+
+### Answer character (qualitative, n=3, no rubric)
+
+On *"Which complexes contain EGFR?"* luna named four specific complexes with
+Reactome links; gpt-4o-mini gave a general description of EGFR signalling naming
+none. Luna's answers cite the retrieved records ("In the supplied Reactome
+records...", "Reactome describes..."); gpt-4o-mini's read as recalled background.
+
+This is the observation that makes the decision interesting, and it is the one with
+the weakest evidence behind it. **A read of three answers is an anecdote.**
+
+### What could not be measured
+
+- **Price per token.** No endpoint reports it. The premise "just as cheap" is
+  unverified here and has to come from the pricing page or from a billing period.
+- **Answer quality at any scale.** See below — the harness that would do it does
+  not currently measure the pipeline that ships.
+
+## The blocker behind the blocker
+
+`src/evaluation/evaluator.py` runs ragas with `faithfulness`, `answer_relevancy`,
+`context_recall` and `context_utilization` over a golden set — exactly the metrics
+that would settle the answer-quality question, and `faithfulness` is exactly the
+axis on which luna appeared to differ.
+
+It cannot be used as it stands. It builds its own retriever — `SelfQueryRetriever`
++ `EnsembleRetriever` + `MergerRetriever` — rather than calling
+`create_bm25_chroma_ensemble_retriever`. Stage 2 removed `SelfQueryRetriever` from
+the shipping pipeline, so the evaluator now measures a configuration that no longer
+exists. Running it today would produce numbers that look like an answer and are not.
+
+This is the same drift `bin/retrieval_baseline` had and had fixed: a measurement
+tool that has quietly stopped measuring the thing.
+
+It also blocks work already owed: the four retrieval changes from the week of
+2026-09-04 (#169, #170, over-fetch, D1) went in **unevaluated for answer quality**.
+Pointing the evaluator at the real pipeline pays for both.
+
+## User Scenarios & Testing *(mandatory)*
+
+### User Story 1 — The evaluator measures what ships (Priority: P1)
+
+A developer changes the model, or the retriever, and runs one command that reports
+faithfulness, answer relevancy, context recall and context utilization for the
+pipeline a user actually talks to.
+
+**Why this priority**: Nothing else here can be decided without it, and it is owed
+regardless of which model wins. It is the only item on this page whose value does
+not depend on the outcome of the decision.
+
+**Independent Test**: Run the evaluator against `main` twice with no change in
+between; the metrics agree within noise. Then swap the retriever's document budget
+and watch context utilization move.
+
+**Acceptance Scenarios**:
+
+1. **Given** the evaluator, **When** it constructs the chain, **Then** it calls the
+   same factory `bin/chat-chainlit.py` reaches, so a change to retrieval cannot
+   alter the product without altering the measurement.
+2. **Given** a golden question set, **When** the evaluator runs on two models,
+   **Then** it emits a per-metric comparison rather than two unrelated reports.
+3. **Given** the evaluator, **When** `SelfQueryRetriever` no longer exists in the
+   pipeline, **Then** the evaluator does not construct one.
+
+---
+
+### User Story 2 — The default model is chosen on evidence (Priority: P2)
+
+The team picks the default from a table of measured differences, not from a
+recollection of three answers.
+
+**Why this priority**: This is the actual question. It is P2 only because it is
+gated on P1.
+
+**Independent Test**: Both models are run over the golden set; the report shows
+faithfulness, relevancy, recall, utilization, latency and token counts side by
+side.
+
+**Acceptance Scenarios**:
+
+1. **Given** both models evaluated, **When** faithfulness differs by less than the
+   run-to-run noise, **Then** the difference is reported as "not measurable", not
+   as a win.
+2. **Given** a chosen default, **When** it is committed, **Then** the reason and
+   the numbers are recorded here, so the next person does not re-derive them.
+
+---
+
+### User Story 3 — Different surfaces may choose differently (Priority: P3)
+
+Chat, chat-alongside-search-results, and analysis summarisation each pick a model
+suited to their tolerance for latency.
+
+**Why this priority**: Only chat exists today. Recording it now costs a paragraph;
+discovering later that the model is a global constant costs a refactor — which is
+the same mistake the context budget made, fixed in Stage 3.
+
+**Acceptance Scenarios**:
+
+1. **Given** a surface that must answer inside a search-results page, **When** it
+   builds its chain, **Then** it can request a faster model without changing what
+   the chat surface uses.
+
+### Edge Cases
+
+- **A model is added to a fixed-temperature family that the prefix table has not
+  met.** It returns a 400 on the first user question, not at startup. `LLM_TEMPERATURE`
+  is the escape hatch; the table is matched on prefix so dated snapshots are covered.
+- **The evaluator's own judge model.** ragas uses an LLM to score. If the judge is
+  the same model being evaluated, the comparison is biased. The judge must be pinned
+  and stated, and must not change between the two runs being compared.
+- **Latency on a streamed interface.** 41s to a complete answer is not 41s of
+  silence if tokens stream; the perceived cost depends on time-to-first-token, which
+  has not been measured and is the number a user actually experiences.
+
+## Requirements *(mandatory)*
+
+### Functional Requirements
+
+- **FR-001**: The evaluator MUST build its chain from the same factory the
+  application uses, so that retrieval changes cannot alter the product without
+  altering the measurement.
+- **FR-002**: The evaluator MUST accept the model under test as an argument and
+  report metrics per model, so two models can be compared in one run.
+- **FR-003**: The evaluator MUST pin and report the judge model separately from the
+  model under test.
+- **FR-004**: The evaluator MUST report run-to-run variance, so a difference smaller
+  than noise is not read as a result.
+- **FR-005**: The default model MUST remain selectable per deployment without a code
+  change.
+- **FR-006**: A model whose temperature requirement is unknown MUST fail with a
+  message naming the environment variable that fixes it.
+- **FR-007**: The chosen default and the numbers behind it MUST be recorded in this
+  specification when the decision is made.
+
+### Key Entities
+
+- **Model under test**: the model that answers, and that the graders use.
+- **Judge model**: the model ragas uses to score. Independent of the above, pinned.
+- **Golden set**: `tests/golden/questions.txt`, 28 questions, already committed and
+  already used by `bin/retrieval_baseline`.
+
+## Success Criteria *(mandatory)*
+
+### Measurable Outcomes
+
+- **SC-001**: One command reports answer quality for the pipeline a user talks to,
+  for a named model, against the golden set.
+- **SC-002**: Two consecutive runs of that command on unchanged code differ by less
+  than the threshold the command itself reports as noise.
+- **SC-003**: The default-model decision is recorded with a per-metric comparison
+  covering both candidates.
+- **SC-004**: Switching the default requires changing one environment variable, and
+  reverting it requires changing it back — no rebuild, no code edit.
+- **SC-005**: The four retrieval changes from 2026-09-04 have an answer-quality
+  measurement attached, closing the gap left open by spec 001.
+
+## Assumptions
+
+- The golden set is representative enough to compare two models. It was assembled
+  for retrieval measurement, not answer grading; if it proves too narrow that is a
+  finding, not a reason to skip the measurement.
+- `gpt-4o-mini` remains available. Nothing here is a migration forced by deprecation.
+- Beta is the place to observe a model change before production. Both pin an image
+  tag, so the model can differ between them by environment alone.
+- Cost is a real constraint but not the binding one at current volume; latency and
+  answer quality are what the team will notice first.
+
+## Decisions for the team
+
+Not gaps in the specification — the P1 work proceeds however these are answered.
+They are recorded so they are answered once, in the open.
+
+### D1 — Does the default flip before or after the quality evaluation?
+
+| option | what it means |
+|---|---|
+| **A. Evaluate first** (recommended) | Fix the evaluator (P1), run both models over the golden set, then decide. Slower; the decision is defensible afterwards. |
+| B. Flip beta now, evaluate alongside | Beta users see luna immediately. Real questions are better than golden ones, but nothing is being recorded, so "it seems better" stays an impression. |
+| C. Flip both now | Fastest. Trades a measurable 2x latency increase on production for an answer-quality improvement supported by three answers. |
+
+**Recommendation: A**, and it costs less than it sounds — the evaluator fix is owed
+anyway for the four unevaluated retrieval changes (SC-005). B is defensible if the
+grounding difference matters more than the latency; C is not, on this evidence.
+
+### D2 — Is ~2x latency acceptable on the chat surface?
+
+22.5s → 41.2s per question, measured to the complete answer. Chainlit streams, so
+what a user feels is time-to-first-token, which has not been measured. If the answer
+is "no", that alone settles the default for chat and D1 becomes moot for that
+surface — though luna may still suit analysis summarisation, where nobody is
+watching a cursor.
+
+### D3 — What is the actual price per token?
+
+The premise for trying luna was "just as cheap". No endpoint reports pricing, so
+this could not be checked here. If luna is materially more expensive per token, the
+larger answers it produces multiply that, and cost becomes a first-order input
+rather than the third one.
+
+## Out of Scope
+
+- The LangChain upgrade. Unblocked by spec 001, unrelated to this.
+- Fine-tuning, or any change to the prompts. Changing prompt and model together
+  would make the comparison unattributable — the same reason spec 001 was staged.
+- Streaming behaviour and time-to-first-token, beyond noting above that it is the
+  number a user feels. Worth its own measurement.