Skip to content

Commit abd54ab

Browse files
ChelseaKRclaude
andcommitted
config: one configuration could produce a grounded answer with no source
`Config(max_passages=0)` returned an answer whose `kind` was "grounded", whose `to_payload()["grounded"]` was true, and which had no sources and no text. Composition slices the accepted passages to `accepted[:max_passages]`, so zero passages were quoted while the trace still said passages had been accepted — the refusal branch never ran, because retrieval had not failed. That is the one outcome this project says it does not have, reached through the constructor instead of through the corpus. `max_passages=-1`, a plausible spelling of "no limit", was quieter and no better: `[:-1]` drops the last accepted passage. `cairn.toml` could not reach either, because `load_config` checked the bounds after building the object. Nothing else did — and this is a reference implementation whose whole invitation is that an agency imports it. There were also no tests over configuration loading at all. The bounds move onto `Config.__post_init__`, so a file and a caller are held to the same rules and there is one place they are stated. `load_config`'s duplicate check is gone. Three tests: the unusable values are refused by both routes, and every usable configuration still grounds on at least one source or refuses with none. No behaviour change for any valid configuration: the bundle re-records byte-identical and the gate returns the same run id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PR8WtSFsAW68eGYW13eb8j
1 parent fcfcabe commit abd54ab

2 files changed

Lines changed: 100 additions & 7 deletions

File tree

cairn/config.py

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,38 @@ class Config:
5757
default_factory=lambda: dict(_DEMO_CONTACTS)
5858
)
5959

60+
def __post_init__(self) -> None:
61+
"""Validate on construction, not only on load.
62+
63+
These bounds used to live in :func:`load_config`, which meant they
64+
held for `cairn.toml` and for nothing else. This is a reference
65+
implementation an agency is invited to import, and
66+
``Config(max_passages=0)`` — a plausible reading of "no limit" —
67+
produced a **grounded answer with no sources and no text**: composition
68+
sliced the accepted passages to nothing while the trace still said
69+
passages had been accepted, so `kind` stayed "grounded" and
70+
`to_payload()["grounded"]` stayed true. That is the one thing this
71+
project says cannot happen, arriving through the constructor rather
72+
than through the corpus. A negative value was quieter and no better:
73+
`accepted[:-1]` silently drops the *last* accepted passage rather than
74+
meaning "all of them".
75+
76+
The invariant is a property of the configuration, so it is enforced
77+
where the configuration is made.
78+
"""
79+
if not 0.0 < self.threshold <= 1.0:
80+
raise ConfigError(
81+
"retrieval.threshold must be in (0, 1]: scores are bounded cosine"
82+
)
83+
if self.max_passages < 1:
84+
raise ConfigError(
85+
"retrieval.max_passages must be >= 1: composing zero passages "
86+
"would emit an answer with no source behind it, which is the "
87+
"one outcome this system does not have"
88+
)
89+
if self.candidates < 1:
90+
raise ConfigError("retrieval.candidates must be >= 1")
91+
6092
def contact_for(self, lang: str) -> str:
6193
"""The human channel a refusal in ``lang`` should point to. Falls back
6294
to the single ``contact`` string, which is what a deployment that
@@ -109,7 +141,9 @@ def load_config(path: str | Path | None = None) -> Config:
109141
language = data.get("language", {})
110142
defaults = Config()
111143
contact = _get(refusal, "contact", str, defaults.contact)
112-
cfg = Config(
144+
# Bounds are checked by Config itself (see __post_init__), so a file and a
145+
# caller cannot be held to two different sets of rules.
146+
return Config(
113147
corpus_path=_get(corpus, "path", str, defaults.corpus_path),
114148
index_path=_get(index, "path", str, defaults.index_path),
115149
threshold=_get(retrieval, "threshold", float, defaults.threshold),
@@ -122,8 +156,3 @@ def load_config(path: str | Path | None = None) -> Config:
122156
contact=contact,
123157
contact_by_language=_contacts(refusal),
124158
)
125-
if not 0.0 < cfg.threshold <= 1.0:
126-
raise ConfigError("retrieval.threshold must be in (0, 1]: scores are bounded cosine")
127-
if cfg.max_passages < 1 or cfg.candidates < 1:
128-
raise ConfigError("retrieval.max_passages and retrieval.candidates must be >= 1")
129-
return cfg

tests/test_answering.py

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@
66
calibration drift silently.
77
"""
88

9+
import tempfile
910
import unittest
1011
from pathlib import Path
1112

12-
from cairn.config import Config
13+
from cairn.config import Config, ConfigError, load_config
1314
from cairn.engine import ask
1415
from cairn.index import build_index
1516
from cairn.retrieve import retrieve
@@ -112,6 +113,69 @@ def test_empty_and_nonsense_queries_refuse(self):
112113
self.assertEqual(self.answer(question).kind, "refusal")
113114

114115

116+
class TestNoConfigurationCanEmitAnUnsourcedAnswer(EngineHarness):
117+
"""The core promise, held against the configuration rather than the corpus.
118+
119+
"There is no code path that emits an answer without supporting corpus
120+
passages" was true of every corpus and every question, and false of one
121+
configuration. `Config(max_passages=0)` — a plausible reading of "no
122+
limit" — sliced the accepted passages away in composition while the trace
123+
still said passages had been accepted, so `kind` stayed `"grounded"`,
124+
`to_payload()["grounded"]` stayed true, and what came back was an empty
125+
answer with no sources under it. A negative value was quieter: `[:-1]`
126+
drops the last accepted passage rather than meaning "all of them".
127+
128+
`cairn.toml` could not reach either, because `load_config` checked the
129+
bounds. Nothing else did, and this is a reference implementation whose
130+
whole invitation is that somebody imports it. The bounds live on `Config`
131+
now, and nothing here had a test.
132+
"""
133+
134+
UNUSABLE = (
135+
("max_passages", 0),
136+
("max_passages", -1),
137+
("candidates", 0),
138+
("threshold", 0.0),
139+
("threshold", 1.5),
140+
)
141+
142+
def test_a_configuration_that_could_break_the_promise_is_refused(self):
143+
for key, value in self.UNUSABLE:
144+
with self.subTest(**{key: value}):
145+
with self.assertRaises(ConfigError):
146+
Config(**{key: value})
147+
148+
def test_the_file_and_the_constructor_are_held_to_the_same_bounds(self):
149+
# Two sets of rules is how the caller-side hole opened in the first
150+
# place: the loader enforced them and the type did not.
151+
for key, value in self.UNUSABLE:
152+
with self.subTest(**{key: value}), tempfile.TemporaryDirectory() as tmp:
153+
path = Path(tmp) / "cairn.toml"
154+
path.write_text(f"[retrieval]\n{key} = {value}\n", encoding="utf-8")
155+
with self.assertRaises(ConfigError):
156+
load_config(path)
157+
158+
def test_every_usable_configuration_grounds_on_at_least_one_source(self):
159+
for cfg in (
160+
Config(),
161+
Config(max_passages=1),
162+
Config(max_passages=8),
163+
Config(candidates=1),
164+
Config(threshold=0.99),
165+
Config(cross_language_fallback=False),
166+
):
167+
for question in (IN_CORPUS[0][0], OFF_TOPIC[0]):
168+
with self.subTest(cfg=cfg.max_passages, question=question):
169+
answer = ask(question, self.index, cfg).answer
170+
payload = answer.to_payload()
171+
self.assertEqual(payload["grounded"], answer.kind == "grounded")
172+
if answer.kind == "grounded":
173+
self.assertTrue(answer.sources, "grounded with nothing under it")
174+
self.assertTrue(answer.text.strip())
175+
else:
176+
self.assertEqual(answer.sources, ())
177+
178+
115179
class TestThresholdCalibration(EngineHarness):
116180
"""The default threshold is a measurement, and this is the measurement."""
117181

0 commit comments

Comments
 (0)