Implement spec 003: the answering model comes from config.yml

.config.schema.yaml

@@ -14,6 +14,30 @@ properties:
             enum: ["all", "logged_in"]
         required: ["enabled"]
     required: ["postprocessing"]
+  llm:
+    # The answering model. Every field is optional; an absent `llm:` section
+    # leaves behaviour exactly as it was. There is deliberately no embedding
+    # model here -- it is derived from the installed bundle, because a query
+    # embedded with a different model than built the vectors returns nonsense
+    # rather than an error.
+    type: object
+    properties:
+      provider:
+        type: string
+        enum: ["openai", "ollama"]
+      model:
+        type: string
+        description: "e.g. gpt-4o-mini, gpt-5.6-luna. LLM_MODEL overrides this."
+      base_url:
+        type: string
+        description: "OpenAI-compatible endpoint, for self-hosted models."
+      temperature:
+        type: number
+        description: >-
+          Almost always leave unset: the value a model requires is derived. Set
+          it only for a model the derived table has not met. A value the model
+          refuses stops the server at startup.
+    additionalProperties: false
   messages:
     type: object
     additionalProperties:

bin/chat-chainlit.py

@@ -47,7 +47,7 @@
 config: Config | None = Config.from_yaml()
 
 profiles: list[ProfileName] = config.profiles if config else [ProfileName.React_to_Me]
-llm_graph = AgentGraph(profiles)
+llm_graph = AgentGraph(profiles, llm_config=config.llm if config else None)
 
 POSTGRES_CHAINLIT_DB = os.getenv("POSTGRES_CHAINLIT_DB")
 S3_BUCKET = os.getenv("S3_BUCKET")

config_default.yml

@@ -3,6 +3,17 @@
 profiles:
   - React-to-Me
 
+# The answering model. Commented out on purpose: with no `llm:` section the
+# built-in default is used, which is what every existing deployment expects.
+# LLM_MODEL in the environment overrides whatever is set here.
+#
+#llm:
+#  provider: openai
+#  model: gpt-4o-mini
+#
+# Do not add an embedding model here. It comes from the bundle that built the
+# vectors; setting it to anything else makes retrieval silently meaningless.
+
 features:
   postprocessing:  # external web search feature
     enabled: true

specs/003-model-configuration/tasks.md

@@ -16,17 +16,17 @@ test asserts the failure. They are written with the code they cover, not after.
 
 ## Phase 1: Setup
 
-- [ ] T001 Create branch `feat/model-configuration` from `origin/main`
-- [ ] T002 Re-read #112 and #151 with `gh pr diff`, to credit them accurately in the commits that land their idea
+- [x] T001 Create branch `feat/model-configuration` from `origin/main`
+- [x] T002 Re-read #112 and #151 with `gh pr diff`, to credit them accurately in the commits that land their idea
 
 ## Phase 2: Foundational
 
 **Blocking: every user story below depends on the config field existing.**
 
-- [ ] T003 Create `LLMConfig` (`provider: str = "openai"`, `model: str | None = None`, `base_url: str | None = None`, `temperature: float | None = None`) in `src/util/config_yml/models.py`, after the shape in #112 and crediting @AaryanCode69
-- [ ] T004 Add `llm: LLMConfig | None = None` to `Config` in `src/util/config_yml/__init__.py` — optional, so a config without it is unchanged (FR-002)
-- [ ] T005 [P] Add the matching `llm` object to `.config.schema.yaml`, with **no** embedding field (FR-004)
-- [ ] T006 [P] Document the section, commented out, in `config_default.yml`
+- [x] T003 Create `LLMConfig` (`provider: str = "openai"`, `model: str | None = None`, `base_url: str | None = None`, `temperature: float | None = None`) in `src/util/config_yml/models.py`, after the shape in #112 and crediting @AaryanCode69
+- [x] T004 Add `llm: LLMConfig | None = None` to `Config` in `src/util/config_yml/__init__.py` — optional, so a config without it is unchanged (FR-002)
+- [x] T005 [P] Add the matching `llm` object to `.config.schema.yaml`, with **no** embedding field (FR-004)
+- [x] T006 [P] Document the section, commented out, in `config_default.yml`
 
 ## Phase 3: User Story 1 — A deployment names its model beside its other settings (P1)
 
@@ -35,13 +35,13 @@ test asserts the failure. They are written with the code they cover, not after.
 **Independent test**: set a model in `config.yml`, start the server, ask a question,
 confirm from the log which model answered. Quickstart steps 1–3.
 
-- [ ] T007 [US1] Add `resolve_llm_model(config)` to `src/agent/graph.py`: `LLM_MODEL` beats `config.llm.model` beats the current default, and document why the precedence is the reverse of `util/secrets.py` (both are "the more specific wins")
-- [ ] T008 [US1] Wire `AgentGraph.__init__` to it, passing `base_url` and `provider` from the config when present, in `src/agent/graph.py`
-- [ ] T009 [US1] Log the effective model at startup in `src/agent/graph.py` (FR-008) — the name only, never a key
-- [ ] T010 [P] [US1] Test in `tests/agent/test_model_configuration.py`: no `llm` section behaves exactly as today (FR-002)
-- [ ] T011 [P] [US1] Test in `tests/agent/test_model_configuration.py`: a configured model is the one selected
-- [ ] T012 [P] [US1] Test in `tests/agent/test_model_configuration.py`: `LLM_MODEL` overrides `config.yml` (FR-003)
-- [ ] T013 [US1] Run quickstart steps 1–3 against a real bundle and confirm the log names the expected model each time (constitution Article I)
+- [x] T007 [US1] Add `resolve_llm_model(config)` to `src/agent/graph.py`: `LLM_MODEL` beats `config.llm.model` beats the current default, and document why the precedence is the reverse of `util/secrets.py` (both are "the more specific wins")
+- [x] T008 [US1] Wire `AgentGraph.__init__` to it, passing `base_url` and `provider` from the config when present, in `src/agent/graph.py`
+- [x] T009 [US1] Log the effective model at startup in `src/agent/graph.py` (FR-008) — the name only, never a key
+- [x] T010 [P] [US1] Test in `tests/agent/test_model_configuration.py`: no `llm` section behaves exactly as today (FR-002)
+- [x] T011 [P] [US1] Test in `tests/agent/test_model_configuration.py`: a configured model is the one selected
+- [x] T012 [P] [US1] Test in `tests/agent/test_model_configuration.py`: `LLM_MODEL` overrides `config.yml` (FR-003)
+- [x] T013 [US1] Run quickstart steps 1–3 against a real bundle and confirm the log names the expected model each time (constitution Article I)
 
 ## Phase 4: User Story 2 — An unusable model stops the server, not the conversation (P1)
 
@@ -50,12 +50,12 @@ confirm from the log which model answered. Quickstart steps 1–3.
 **Independent test**: `gpt-5.6-luna` with `temperature: 0` must refuse to start.
 Quickstart steps 4–5.
 
-- [ ] T014 [US2] Extend `resolve_temperature` in `src/agent/graph.py` to accept a configured temperature and raise `SystemExit` naming model, value and fix when the model refuses it (FR-006)
-- [ ] T015 [P] [US2] Test in `tests/agent/test_model_temperature.py`: luna + `temperature: 0` exits, and the message contains all three of model, value and remedy
-- [ ] T016 [P] [US2] Test in `tests/agent/test_model_temperature.py`: a model absent from the table starts normally (FR-007)
-- [ ] T017 [P] [US2] Test in `tests/agent/test_model_temperature.py`: `LLM_TEMPERATURE` still wins over the configured value
-- [ ] T018 [US2] Perturbation check: delete the guard and confirm T015 fails — a test that cannot fail is not a tripwire (Article III)
-- [ ] T019 [US2] Run quickstart steps 4–5 and confirm the server refuses to start rather than failing on the first question
+- [x] T014 [US2] Extend `resolve_temperature` in `src/agent/graph.py` to accept a configured temperature and raise `SystemExit` naming model, value and fix when the model refuses it (FR-006)
+- [x] T015 [P] [US2] Test in `tests/agent/test_model_temperature.py`: luna + `temperature: 0` exits, and the message contains all three of model, value and remedy
+- [x] T016 [P] [US2] Test in `tests/agent/test_model_temperature.py`: a model absent from the table starts normally (FR-007)
+- [x] T017 [P] [US2] Test in `tests/agent/test_model_temperature.py`: `LLM_TEMPERATURE` still wins over the configured value
+- [x] T018 [US2] Perturbation check: delete the guard and confirm T015 fails — a test that cannot fail is not a tripwire (Article III)
+- [x] T019 [US2] Run quickstart steps 4–5 and confirm the server refuses to start rather than failing on the first question
 
 ## Phase 5: User Story 3 — Surfaces choose their own model (P2)
 
@@ -68,9 +68,9 @@ Quickstart steps 4–5.
 
 ## Phase 6: Polish & Cross-Cutting
 
-- [ ] T022 [P] Verify `grep -rn embedding .config.schema.yaml config_default.yml` finds no embedding model field (SC-004), and add a test asserting it
-- [ ] T023 [P] Confirm `tests/util/test_config.py` passes **untouched** — adding a section must not change what an invalid config does (Article III)
-- [ ] T024 Run `ruff check`, `ruff format --check`, `mypy`, `pytest`
+- [x] T022 [P] Verify `grep -rn embedding .config.schema.yaml config_default.yml` finds no embedding model field (SC-004), and add a test asserting it
+- [x] T023 [P] Confirm `tests/util/test_config.py` passes **untouched** — adding a section must not change what an invalid config does (Article III)
+- [x] T024 Run `ruff check`, `ruff format --check`, `mypy`, `pytest`
 - [ ] T025 Close #112 with credit to @AaryanCode69, stating plainly that the LLM half is harvested and the embedding half rejected because it bypasses `resolve_embedding_model()` and would silently break Plant Reactome
 - [ ] T026 Close #151 with credit to @bhavyakeerthi3, noting the flat-string shape was reasonable but `base_url` has nowhere to live in it
 - [ ] T027 Update `specs/003-model-configuration/spec.md` with the outcome, and record D1 as taken-as-recommended

src/agent/graph.py

@@ -17,6 +17,7 @@
 from agent.models import get_embedding, get_llm
 from agent.profiles import ProfileName, create_profile_graphs
 from agent.profiles.base import InputState, OutputState
+from util.config_yml.models import LLMConfig
 from util.embedding_environment import EmbeddingEnvironment
 from util.logging import logging
 from util.secrets import get_db_uri
@@ -126,38 +127,93 @@ def resolve_embedding_model() -> str:
 _SNAPSHOT_SUFFIX = re.compile(r"-\d{4}-\d{2}-\d{2}$")
 
 
-def resolve_temperature(model: str) -> float:
+def refuses_zero(model: str) -> bool:
+    """Whether `model` accepts only its own default temperature."""
+    return _SNAPSHOT_SUFFIX.sub("", model) in FIXED_TEMPERATURE_MODELS
+
+
+def resolve_temperature(model: str, *, configured: float | None = None) -> float:
     """The temperature to send for `model`.
 
     Returning 1.0 for the models above trades determinism for being able to use
     them at all. That trade is made here, once, rather than at each call site.
+
+    A `configured` value that the model is known to refuse stops the process.
+    Without that check the mistake surfaces as a 400 on a user's first question
+    -- visible to a user, attributed to the chatbot, and diagnosable only from
+    logs. A model the table has not met is not validated at all: the table is
+    empirical and always behind, so an unknown model must not block startup.
     """
     override = os.getenv("LLM_TEMPERATURE")
     if override is not None and override.strip() != "":
         try:
             return float(override)
         except ValueError:
             raise SystemExit(f"LLM_TEMPERATURE={override!r} is not a number.") from None
-    if _SNAPSHOT_SUFFIX.sub("", model) in FIXED_TEMPERATURE_MODELS:
-        return FIXED_TEMPERATURE
-    return 0.0
+
+    if configured is not None:
+        if refuses_zero(model) and configured != FIXED_TEMPERATURE:
+            raise SystemExit(
+                f"config.yml sets llm.temperature={configured} for {model!r}, "
+                f"which accepts only {FIXED_TEMPERATURE}. Remove the temperature "
+                "and it will be derived, or set LLM_TEMPERATURE to override."
+            )
+        return configured
+
+    return FIXED_TEMPERATURE if refuses_zero(model) else 0.0
+
+
+DEFAULT_LLM_MODEL = "gpt-4o-mini"
+
+
+def resolve_llm_model(llm_config: "LLMConfig | None") -> tuple[str, str, str | None]:
+    """Pick the answering model: environment, then config.yml, then the default.
+
+    Returns (provider, model, base_url).
+
+    Environment beats file here, which is the reverse of util/secrets.py, where a
+    mounted Docker secret beats the environment. The two are the same rule seen
+    from different sides -- the more specific source wins. A secret is mounted BY
+    a deployment and should beat a file committed to the repository; LLM_MODEL is
+    how an operator overrides a committed config.yml for one container without
+    editing it. Stating this because the inconsistency looks like a bug until you
+    see which way each one points.
+    """
+    provider = "openai"
+    model = DEFAULT_LLM_MODEL
+    base_url = os.getenv("LLM_BASE_URL")
+
+    if llm_config is not None:
+        provider = llm_config.provider
+        model = llm_config.model or model
+        base_url = base_url or llm_config.base_url
+
+    return provider, os.getenv("LLM_MODEL", model), base_url
 
 
 class AgentGraph:
     def __init__(
         self,
         profiles: list[ProfileName],
+        llm_config: "LLMConfig | None" = None,
     ) -> None:
         # Get base models
         embedding_model = resolve_embedding_model()
-        llm_model = os.getenv("LLM_MODEL", "gpt-4o-mini")
-        llm_base_url = os.getenv("LLM_BASE_URL", None)
+        llm_provider, llm_model, llm_base_url = resolve_llm_model(llm_config)
+        temperature = resolve_temperature(
+            llm_model, configured=llm_config.temperature if llm_config else None
+        )
+        # The name only. A model id is not a secret, but this is the line a key
+        # would end up on if anyone ever widened it.
+        logging.info(
+            f"Answering with {llm_provider}/{llm_model} at temperature {temperature}"
+        )
         llm: BaseChatModel = get_llm(
-            "openai",
+            llm_provider,
             llm_model,
             base_url=llm_base_url,
             request_timeout=360.0,
-            temperature=resolve_temperature(llm_model),
+            temperature=temperature,
         )
         embedding_base_url = os.getenv("OPENAI_BASE_URL", None)
         embedding: Embeddings = get_embedding(

src/util/config_yml/__init__.py

@@ -2,11 +2,12 @@
 from typing import Self
 
 import yaml
-from pydantic import BaseModel, ValidationError
+from pydantic import BaseModel, ConfigDict, ValidationError
 
 from agent.profile_names import ProfileName
 from util.config_yml.features import Feature, Features
 from util.config_yml.messages import Message, TriggerEvent
+from util.config_yml.models import LLMConfig
 from util.config_yml.usage_limits import MessageRate, UsageLimits
 from util.config_yml.user_matching import match_user
 from util.logging import logging
@@ -20,7 +21,20 @@
 
 
 class Config(BaseModel):
+    # extra="forbid" for the same reason LLMConfig does it, one level up. Without
+    # it a typo in a section name -- `llmm:` for `llm:`, or a key at the wrong
+    # indentation -- loads cleanly, does nothing, and leaves the operator
+    # believing they configured something. Checked against config.yml and
+    # config_default.yml before turning on: neither carries an unknown key, so
+    # this refuses nothing that works today.
+    model_config = ConfigDict(extra="forbid")
+
     features: Features
+    # Optional, and None rather than a default instance: a config.yml with no
+    # `llm:` section must behave exactly as it did before this field existed
+    # (spec 003 FR-002), and "absent" has to be distinguishable from "present
+    # and empty" for that to hold.
+    llm: LLMConfig | None = None
     messages: dict[str, Message]
     profiles: list[ProfileName]
     usage_limits: UsageLimits

src/util/config_yml/models.py

@@ -0,0 +1,45 @@
+"""Which model a deployment answers with.
+
+The shape is @AaryanCode69's from #112 -- named fields rather than a
+"provider/model" string to re-parse, which is also where `base_url` can live.
+Plant Reactome needs that: it serves its embedding model from a self-hosted
+OpenAI-compatible endpoint.
+
+What is deliberately absent is an embedding model. It is derived from the bundle
+that built the vectors (`agent.graph.resolve_embedding_model`), because a query
+embedded with a different model than the stored vectors returns nonsense rather
+than an error. Both #112 and #151 made it configurable; that is the one part of
+them not taken. See specs/003-model-configuration/spec.md FR-004.
+"""
+
+from pydantic import BaseModel, ConfigDict
+
+
+class LLMConfig(BaseModel):
+    """The answering model. Every field is optional, so an `llm:` section may set
+    only what it wants to change and inherit the rest."""
+
+    # extra="forbid" so an unknown key is a validation error, which Config.from_yaml
+    # treats as fatal. Pydantic's default is to ignore extras silently -- meaning a
+    # config.yml saying `embedding_model: text-embedding-3-large` would be accepted,
+    # discarded, and leave an operator believing they had set it. Refusing to start
+    # is the only honest answer to a setting that cannot be honoured.
+    model_config = ConfigDict(extra="forbid")
+
+    provider: str = "openai"
+
+    # None means "not configured here", which leaves LLM_MODEL and then the
+    # built-in default in charge. A default of "gpt-4o-mini" would instead make
+    # every config.yml silently pin that model, which is the opposite of
+    # FR-002's promise that an absent section changes nothing.
+    model: str | None = None
+
+    base_url: str | None = None
+
+    # Almost always leave unset. The value a model requires is derived in
+    # agent.graph.resolve_temperature from a measured table, because it is a
+    # property of the model rather than a preference: the gpt-5.5/5.6 families,
+    # o3 and o4-mini accept only 1.0, while gpt-5.1/5.2/5.4 accept 0.0. Setting
+    # it here to something the model refuses now stops startup rather than
+    # failing on a user's first question (FR-006).
+    temperature: float | None = None