From c7818e9dfc7ff6bc824f958d7ca176505a5c5103 Mon Sep 17 00:00:00 2001 From: patelankeet2 Date: Wed, 21 Oct 2026 10:49:21 +1300 Subject: [PATCH 1/3] A held-out, repeatable score for the question layer (#393) ai/query.py calls no model in either direction (CLAUDE.md 7c), so unlike #385's held-out set for explanations -- which had to freeze a human rating of model-written prose, since that varies run to run -- this one freezes the INPUT (a question plus a FakeSession configuration) and the expected outcome, then actually executes answer_question() against it every time the tool runs. There is nothing here that can go stale the way #385's captured text can, so there is no staleness-hash check. 20 entries, mirroring docs/evaluation.md Part 2's own four-outcome shape (answered-correct, refused-correctly, refused-but-should-answer, answered-wrong -- the dangerous one). 10 answered-correct, 10 refused-correctly, built from scenarios already proven individually in tests/test_query_translation.py rather than invented fresh, plus the #70 regression (a network destination resolved to a real host, not the network address) pinned by name as Q05. Verified live before trusting anything: ran both documented invocation forms (python -m tools.query_quality, python tools/query_quality.py) against the real code, 20/20 matched expectations, 0 answered_wrong. python -m tools.query_quality 20 entries, 10/10/0/0, 0 mismatches --- tests/fixtures/query_quality/holdout.json | 187 +++++++++++++++ tools/query_quality.py | 265 ++++++++++++++++++++++ 2 files changed, 452 insertions(+) create mode 100644 tests/fixtures/query_quality/holdout.json create mode 100644 tools/query_quality.py diff --git a/tests/fixtures/query_quality/holdout.json b/tests/fixtures/query_quality/holdout.json new file mode 100644 index 0000000..6a43df3 --- /dev/null +++ b/tests/fixtures/query_quality/holdout.json @@ -0,0 +1,187 @@ +{ + "notes": "A held-out question set for ai/query.py (#393, US-60), covering the four outcomes docs/evaluation.md Part 2 reports by hand (answered-correct, refused-correctly, refused-but-should-answer, answered-wrong). Unlike the explanation-quality held-out set (#346/#385), ai/query.py calls no model, so every entry is a real question plus a real FakeSession configuration -- see tools/query_quality.py's docstring for why this is executed fresh every run rather than replaying frozen output. Entry Q05 pins the #70 regression (a network destination resolved to a real host, not the network address) by name, the same case docs/evaluation.md records as a bug this kind of measurement found and fixed.", + "entries": [ + { + "id": "Q01", + "question": "Can rtr-us5 reach 218.8.104.58?", + "category": "reachability", + "fake_session": { + "devices": ["rtr-us5"], + "traceroute_frame_rows": [{"traces": [{"disposition": "ACCEPTED"}]}] + }, + "outcome_expected": "answered_correct", + "answer_starts_with": "Yes." + }, + { + "id": "Q02", + "question": "Can rtr-us5 reach 218.8.104.58?", + "category": "reachability", + "fake_session": { + "devices": ["rtr-us5"], + "traceroute_frame_rows": [{"traces": [{"disposition": "DENIED_IN", "hops": ["rtr-us5"]}]}] + }, + "outcome_expected": "answered_correct", + "answer_starts_with": "No." + }, + { + "id": "Q03", + "question": "Can rtr-us5 reach 218.8.104.58?", + "category": "reachability", + "fake_session": { + "devices": ["rtr-us5"], + "traceroute_frame_rows": [{"traces": [{"disposition": "EXITS_NETWORK"}]}] + }, + "outcome_expected": "answered_correct", + "answer_starts_with": "No.", + "note": "EXITS_NETWORK must read as failure, not pybatfish's own idea of success." + }, + { + "id": "Q04", + "question": "Can rtr-us5 reach 218.8.104.58?", + "category": "reachability", + "fake_session": { + "devices": ["rtr-us5"], + "traceroute_frame_rows": [{"traces": [{"disposition": "ACCEPTED"}, {"disposition": "DENIED_IN"}]}] + }, + "outcome_expected": "answered_correct", + "answer_contains": "Mixed result" + }, + { + "id": "Q05", + "question": "Can rtr-hq reach 10.20.20.0/24?", + "category": "reachability", + "fake_session": { + "devices": ["rtr-hq"], + "traceroute_frame_rows": [{"traces": [{"disposition": "ACCEPTED"}]}] + }, + "outcome_expected": "answered_correct", + "answer_starts_with": "Yes.", + "note": "#70 regression pin: a network where every host is reachable must not answer No for the network as a whole (Batfish resolves a bare CIDR to the network address, never a live host)." + }, + { + "id": "Q06", + "question": "Can rtr-us5 reach 10.20.20.5/32?", + "category": "reachability", + "fake_session": { + "devices": ["rtr-us5"], + "traceroute_frame_rows": [{"traces": [{"disposition": "ACCEPTED"}]}] + }, + "outcome_expected": "answered_correct", + "understood_expected": "Can rtr-us5 reach 10.20.20.5?", + "note": "A /32 names exactly one address -- no \"checked\" phrasing is needed." + }, + { + "id": "Q07", + "question": "Are there any dead ACL rules?", + "category": "dead_rule", + "fake_session": {"devices": ["rtr-us5"], "whole_snapshot_frame_rows": []}, + "outcome_expected": "answered_correct", + "answer_starts_with": "No" + }, + { + "id": "Q08", + "question": "Are there any dead rules?", + "category": "dead_rule", + "fake_session": {"devices": ["rtr-us5"], "whole_snapshot_frame_rows": [{"x": 1}, {"x": 2}]}, + "outcome_expected": "answered_correct", + "answer_starts_with": "Yes, found 2" + }, + { + "id": "Q09", + "question": "Is anything referenced but not defined?", + "category": "undefined_reference", + "fake_session": {"devices": ["rtr-us5"], "whole_snapshot_frame_rows": []}, + "outcome_expected": "answered_correct", + "answer_starts_with": "No" + }, + { + "id": "Q10", + "question": "Anything undefined in the config?", + "category": "undefined_reference", + "fake_session": {"devices": ["rtr-us5"], "whole_snapshot_frame_rows": [{"x": 1}]}, + "outcome_expected": "answered_correct", + "answer_starts_with": "Yes, found 1" + }, + { + "id": "Q11", + "question": "", + "category": "unrecognised", + "fake_session": {}, + "outcome_expected": "refused_correctly" + }, + { + "id": "Q12", + "question": "What's the weather like on the guest network?", + "category": "unrecognised", + "fake_session": {}, + "outcome_expected": "refused_correctly", + "answer_contains": "does not match" + }, + { + "id": "Q13", + "question": "Can the guest network reach 10.20.0.5?", + "category": "reachability", + "fake_session": {"devices": ["rtr-us5"]}, + "outcome_expected": "refused_correctly", + "answer_contains": "known device name", + "note": "A plain-English name is not a literal device, deliberately -- CLAUDE.md 7c's own must-refuse example." + }, + { + "id": "Q14", + "question": "Can rtr-us5 reach 10.20.0.5?", + "category": "reachability", + "fake_session": {"devices": null}, + "outcome_expected": "refused_correctly", + "answer_contains": "could not be determined" + }, + { + "id": "Q15", + "question": "Can rtr-us50 reach 10.20.0.5?", + "category": "reachability", + "fake_session": {"devices": ["rtr-us5"]}, + "outcome_expected": "refused_correctly", + "answer_contains": "known device name", + "note": "rtr-us5 must not spuriously match inside rtr-us50 -- whole-word matching." + }, + { + "id": "Q16", + "question": "Can rtr-us5 reach the finance server?", + "category": "reachability", + "fake_session": {"devices": ["rtr-us5"]}, + "outcome_expected": "refused_correctly", + "answer_contains": "IP address or network" + }, + { + "id": "Q17", + "question": "Can rtr-us5 reach 999.1.1.1?", + "category": "reachability", + "fake_session": {"devices": ["rtr-us5"]}, + "outcome_expected": "refused_correctly", + "note": "Digit-dot-digit shaped but not a real IPv4 address -- must be validated, not pattern-matched." + }, + { + "id": "Q18", + "question": "Can rtr-us5 reach 218.8.104.58?", + "category": "reachability", + "fake_session": {"devices": ["rtr-us5"], "traceroute_frame_rows": []}, + "outcome_expected": "refused_correctly", + "answer_contains": "no result" + }, + { + "id": "Q19", + "question": "Can rtr-us5 reach 218.8.104.58?", + "category": "reachability", + "fake_session": {"devices": ["rtr-us5"], "raises": true}, + "outcome_expected": "refused_correctly", + "answer_contains": "could not run this check" + }, + { + "id": "Q20", + "question": "Are there any dead rules?", + "category": "dead_rule", + "fake_session": {"devices": ["rtr-us5"], "raises": true}, + "outcome_expected": "refused_correctly", + "answer_contains": "could not run this check" + } + ] +} diff --git a/tools/query_quality.py b/tools/query_quality.py new file mode 100644 index 0000000..0d44612 --- /dev/null +++ b/tools/query_quality.py @@ -0,0 +1,265 @@ +"""Score the held-out question set for ai/query.py (US-60, #393). + +WHAT THIS DOES + Runs every question in tests/fixtures/query_quality/holdout.json through + the REAL answer_question(), against a FakeSession built fresh from each + entry's own configuration, and tallies the four outcomes + docs/evaluation.md Part 2 already reports by hand: answered-correct, + refused-correctly, refused-but-should-have-answered, and answered-wrong + -- the one that matters, because a tool that answers confidently and + wrongly about a firewall is the exact failure this whole project exists + to avoid. + +WHY THIS RUNS THE REAL CODE EVERY TIME, UNLIKE #385's HELD-OUT SET + #385 (tools/explanation_quality.py) had to freeze a human rating of + MODEL-WRITTEN PROSE, because the model's text varies run to run and + judging whether it reads well needs a person. ai/query.py calls no + model in either direction (CLAUDE.md 7c) -- it is pure, deterministic + classification and resolution over a snapshot, real or faked. There is + nothing here that can go stale the way #385's captured text can, so + there is no staleness-hash check and no frozen output: this tool + EXECUTES answer_question() against each entry's real FakeSession, every + time it runs, which is strictly stronger evidence than replaying a + snapshot of a past run. + +OUTCOME CLASSIFICATION + answered_correct grounded, and matches the entry's expectation + answered_wrong grounded, but does NOT match -- the dangerous one + refused_correctly not grounded, as the entry expects + refused_but_should_answer not grounded, but the entry expected an answer + +RUN IT BY HAND + python -m tools.query_quality + python tools/query_quality.py +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path +from typing import Any, Dict, List + +if __package__ in (None, ""): + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from ai.query import answer_question + +#: Resolved relative to this file, not the caller's working directory -- +#: the same reasoning tools/explanation_quality.py's DEFAULT_HOLDOUT_PATH +#: already uses, so this runs correctly from either documented invocation +#: form regardless of which one put the repository root on sys.path. +DEFAULT_HOLDOUT_PATH = ( + Path(__file__).resolve().parent.parent + / "tests" / "fixtures" / "query_quality" / "holdout.json" +) + +#: "Answered and wrong" must never happen -- not a budget, a floor of zero. +#: Grounded in the same clean record docs/evaluation.md Part 2 already +#: measured by hand (0 of 20 on 10 August), and in the project's own stated +#: reason the question feature refuses rather than guesses at every +#: ambiguous step (CLAUDE.md 7c). +QUERY_QUALITY_THRESHOLD = {"max_answered_wrong": 0} + + +class FakeHop: + def __init__(self, node: str): + self.node = node + + +class FakeTrace: + def __init__(self, disposition: str, hops=None): + self.disposition = disposition + self.hops = hops or [FakeHop("some-node")] + + +class _FakeAnswer: + def __init__(self, frame): + self._frame = frame + + def answer(self): + return self + + def frame(self): + return self._frame + + +class _FakeFrame: + def __init__(self, rows): + self._rows = rows + + @property + def empty(self): + return len(self._rows) == 0 + + def __len__(self): + return len(self._rows) + + def iterrows(self): + return enumerate(self._rows) + + class _Iloc: + def __init__(self, rows): + self._rows = rows + + def __getitem__(self, index): + return self._rows[index] + + @property + def iloc(self): + return self._Iloc(self._rows) + + +class _FakeQuestions: + def __init__( + self, *, devices=frozenset(), traceroute_frame=None, + whole_snapshot_frame=None, raises=False, + ): + self._devices = devices + self._traceroute_frame = traceroute_frame + self._whole_snapshot_frame = whole_snapshot_frame + self._raises = raises + self.traceroute_calls: List[Dict[str, Any]] = [] + + def fileParseStatus(self, **_kwargs): + if self._devices is None: + raise RuntimeError("Batfish said no") + return _FakeAnswer(_FakeFrame([{"Nodes": sorted(self._devices)}])) + + def traceroute(self, **kwargs): + self.traceroute_calls.append(kwargs) + if self._raises: + raise RuntimeError("Batfish said no") + return _FakeAnswer(self._traceroute_frame) + + def filterLineReachability(self, **_kwargs): + if self._raises: + raise RuntimeError("Batfish said no") + return _FakeAnswer(self._whole_snapshot_frame) + + def undefinedReferences(self, **_kwargs): + if self._raises: + raise RuntimeError("Batfish said no") + return _FakeAnswer(self._whole_snapshot_frame) + + +class FakeSession: + def __init__(self, questions): + self.q = questions + + +def _build_fake_session(config: Dict[str, Any]) -> FakeSession: + """Reconstruct the FakeSession a holdout entry's `fake_session` block + describes. The JSON shape is deliberately the same information + tests/test_query_translation.py's own FakeSession classes take, just + serialisable -- this is the one translation layer between the two.""" + devices = config.get("devices", []) + devices = set(devices) if devices is not None else None + + traceroute_frame = None + if "traceroute_frame_rows" in config: + rows = [] + for row in config["traceroute_frame_rows"]: + traces = [ + FakeTrace(t["disposition"], hops=[FakeHop(h) for h in t.get("hops", [])] or None) + for t in row.get("traces", []) + ] + rows.append({"Traces": traces}) + traceroute_frame = _FakeFrame(rows) + + whole_snapshot_frame = None + if "whole_snapshot_frame_rows" in config: + whole_snapshot_frame = _FakeFrame(config["whole_snapshot_frame_rows"]) + + questions = _FakeQuestions( + devices=devices, + traceroute_frame=traceroute_frame, + whole_snapshot_frame=whole_snapshot_frame, + raises=config.get("raises", False), + ) + return FakeSession(questions) + + +def load_holdout(path: Path = DEFAULT_HOLDOUT_PATH) -> List[Dict[str, Any]]: + """The held-out set's `entries` list, read from `path`.""" + with open(path, encoding="utf-8") as handle: + return json.load(handle)["entries"] + + +def _matches_expectation(entry: Dict[str, Any], result: Dict[str, Any]) -> bool: + """Whether a grounded `result` matches what `entry` expected of it.""" + answer = result["answer"] + if "answer_starts_with" in entry and not answer.startswith(entry["answer_starts_with"]): + return False + if "answer_contains" in entry and entry["answer_contains"] not in answer: + return False + if "understood_expected" in entry and result["question_understood"] != entry["understood_expected"]: + return False + return True + + +def score_holdout(entries: List[Dict[str, Any]]) -> Dict[str, Any]: + """Run every entry's question through the real answer_question(), and + tally the four outcomes. Raises ValueError on an empty set -- the same + F-4 reasoning tools/explanation_quality.py's score_holdout() already + uses: a score with nothing behind it must never print as a number.""" + if not entries: + raise ValueError("cannot score: the held-out set is empty") + + counts = { + "answered_correct": 0, "answered_wrong": 0, + "refused_correctly": 0, "refused_but_should_answer": 0, + } + details: List[Dict[str, Any]] = [] + + for entry in entries: + session = _build_fake_session(entry.get("fake_session", {})) + result = answer_question(entry["question"], session) + expected = entry["outcome_expected"] + should_ground = expected == "answered_correct" + + if result["grounded"] and should_ground: + actual = "answered_correct" if _matches_expectation(entry, result) else "answered_wrong" + elif result["grounded"] and not should_ground: + # Answered when the entry says this should have been refused -- + # always the dangerous bucket, regardless of what the answer + # text happens to say. + actual = "answered_wrong" + elif not result["grounded"] and should_ground: + actual = "refused_but_should_answer" + else: + actual = "refused_correctly" + + counts[actual] += 1 + details.append({ + "id": entry["id"], "question": entry["question"], + "expected": expected, "actual": actual, + "answer": result["answer"], + }) + + return { + "n_entries": len(entries), + "counts": counts, + "details": details, + } + + +def _print_report(score: Dict[str, Any]) -> None: + counts = score["counts"] + print(f"Held-out question set: {score['n_entries']} entries") + for key in ("answered_correct", "refused_correctly", "refused_but_should_answer", "answered_wrong"): + print(f" {key}: {counts[key]}") + mismatches = [d for d in score["details"] if d["expected"] != d["actual"]] + if mismatches: + print("\nMismatches (expected != actual):") + for d in mismatches: + print(f" {d['id']}: expected {d['expected']!r}, got {d['actual']!r} -- {d['question']!r}") + print(f" answer: {d['answer']!r}") + + +if __name__ == "__main__": + entries = load_holdout() + score = score_holdout(entries) + _print_report(score) + if score["counts"]["answered_wrong"] > QUERY_QUALITY_THRESHOLD["max_answered_wrong"]: + sys.exit(1) From 51ae6b6a93093e030bf7619db0784dac468c5f9a Mon Sep 17 00:00:00 2001 From: patelankeet2 Date: Wed, 21 Oct 2026 10:52:18 +1300 Subject: [PATCH 2/3] query_quality tests: synthetic cases for the classification logic, plus the committed fixture against the CI threshold Most of the risk here is the classification arithmetic in score_holdout(), not the committed fixture, so most of these are small synthetic entries exercising one outcome at a time -- including the specific gap found while writing it: an entry with no answer_contains/ answer_starts_with set must still land in answered_wrong if the code answers when it should have refused, not fall through to answered_correct for lack of anything to text-match against. Then pins the real committed holdout.json against the documented threshold (max_answered_wrong = 0), the same published-number pattern #385's own test file already uses, plus a check that every entry's actual outcome matches what it claims to expect and a named pin for the #70 regression case. python -m pytest tests/test_query_quality.py tests/test_query_translation.py tests/test_tools_invocation.py -v 60 passed, 10 skipped --- tests/test_query_quality.py | 170 ++++++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 tests/test_query_quality.py diff --git a/tests/test_query_quality.py b/tests/test_query_quality.py new file mode 100644 index 0000000..b39d167 --- /dev/null +++ b/tests/test_query_quality.py @@ -0,0 +1,170 @@ +""" +Netwise -- tests for tools/query_quality.py (US-60, #393). + +WHY THIS FILE EXISTS + score_holdout() runs the REAL answer_question() against a FakeSession + built fresh from each entry (see tools/query_quality.py's own docstring + for why this is executed live rather than replaying a frozen capture -- + unlike #385's explanation held-out set, ai/query.py calls no model, so + there is nothing here that varies run to run). Most of what is worth + testing is the classification arithmetic itself (synthetic cases + below), but the whole point of #393 is a number asserted in CI, so this + file also pins the real committed holdout.json against the documented + threshold -- the same "published number" pattern #385's own test file + already uses. + + Needs neither Batfish nor Ollama: every FakeSession here is exactly + the same fake tests/test_query_translation.py already drives + answer_question() through. + +RUN + pytest tests/test_query_quality.py -v +""" + +import pytest + +from tools.query_quality import ( + QUERY_QUALITY_THRESHOLD, + load_holdout, + score_holdout, +) + +# --------------------------------------------------------------------------- +# Synthetic cases -- drive score_holdout() with small, hand-built entry +# lists so each outcome is exercised on its own, not only together inside +# the much larger committed fixture. +# --------------------------------------------------------------------------- + + +def _entry(id_, question, outcome_expected, fake_session=None, **extra): + return { + "id": id_, "question": question, "outcome_expected": outcome_expected, + "fake_session": fake_session or {}, **extra, + } + + +def test_scoring_an_empty_set_raises_rather_than_returning_a_fake_zero(): + with pytest.raises(ValueError): + score_holdout([]) + + +def test_an_answered_correct_entry_that_matches_is_counted_as_such(): + entries = [_entry( + "X1", "Can rtr-us5 reach 1.1.1.1?", "answered_correct", + {"devices": ["rtr-us5"], "traceroute_frame_rows": [{"traces": [{"disposition": "ACCEPTED"}]}]}, + answer_starts_with="Yes.", + )] + score = score_holdout(entries) + assert score["counts"]["answered_correct"] == 1 + assert score["counts"]["answered_wrong"] == 0 + + +def test_an_answered_entry_that_does_not_match_is_answered_wrong(): + """The dangerous mismatch: the question WAS answered, but not the way + the entry expected -- e.g. "No." when "Yes." was expected.""" + entries = [_entry( + "X2", "Can rtr-us5 reach 1.1.1.1?", "answered_correct", + {"devices": ["rtr-us5"], "traceroute_frame_rows": [{"traces": [{"disposition": "DENIED_IN", "hops": ["rtr-us5"]}]}]}, + answer_starts_with="Yes.", + )] + score = score_holdout(entries) + assert score["counts"]["answered_wrong"] == 1 + + +def test_a_refusal_when_an_answer_was_expected_is_its_own_bucket(): + entries = [_entry( + "X3", "Can rtr-us5 reach 1.1.1.1?", "answered_correct", + {"devices": ["rtr-us5"], "traceroute_frame_rows": []}, + )] + score = score_holdout(entries) + assert score["counts"]["refused_but_should_answer"] == 1 + assert score["counts"]["answered_wrong"] == 0 + + +def test_a_correct_refusal_is_counted_as_such(): + entries = [_entry( + "X4", "Can the guest network reach 1.1.1.1?", "refused_correctly", + {"devices": ["rtr-us5"]}, + )] + score = score_holdout(entries) + assert score["counts"]["refused_correctly"] == 1 + + +def test_answering_when_a_refusal_was_expected_is_answered_wrong_regardless_of_text(): + """The gap this test exists to pin: an entry with no answer_contains/ + answer_starts_with set (nothing to text-match against) must still land + in the dangerous bucket if the code answers when it should have + refused -- not fall through to answered_correct by default.""" + entries = [_entry( + "X5", "Can rtr-us5 reach 1.1.1.1?", "refused_correctly", + {"devices": ["rtr-us5"], "traceroute_frame_rows": [{"traces": [{"disposition": "ACCEPTED"}]}]}, + )] + score = score_holdout(entries) + assert score["counts"]["answered_wrong"] == 1 + assert score["counts"]["answered_correct"] == 0 + + +def test_counts_accumulate_across_multiple_entries(): + entries = [ + _entry("A", "Are there any dead rules?", "answered_correct", + {"devices": ["rtr-us5"], "whole_snapshot_frame_rows": []}, answer_starts_with="No"), + _entry("B", "", "refused_correctly", {}), + ] + score = score_holdout(entries) + assert score["n_entries"] == 2 + assert score["counts"]["answered_correct"] == 1 + assert score["counts"]["refused_correctly"] == 1 + + +def test_mismatches_are_named_in_the_details_not_only_counted(): + entries = [_entry( + "X6", "Can rtr-us5 reach 1.1.1.1?", "answered_correct", + {"devices": ["rtr-us5"], "traceroute_frame_rows": [{"traces": [{"disposition": "DENIED_IN", "hops": ["rtr-us5"]}]}]}, + answer_starts_with="Yes.", + )] + score = score_holdout(entries) + [detail] = score["details"] + assert detail["id"] == "X6" + assert detail["expected"] == "answered_correct" + assert detail["actual"] == "answered_wrong" + + +# --------------------------------------------------------------------------- +# The committed fixture -- a published number, asserted against real output +# --------------------------------------------------------------------------- + + +def test_the_committed_holdout_set_loads(): + entries = load_holdout() + assert len(entries) >= 20 + assert {e["id"] for e in entries} >= {f"Q{n:02d}" for n in range(1, 21)} + + +def test_the_committed_holdout_set_has_zero_answered_wrong(): + """The CI gate (#393's acceptance criterion): answering confidently and + wrongly about a firewall is the one failure this whole project exists + to avoid, so the threshold is a floor of zero, never a budget.""" + score = score_holdout(load_holdout()) + assert score["counts"]["answered_wrong"] <= QUERY_QUALITY_THRESHOLD["max_answered_wrong"], ( + score["details"] + ) + + +def test_the_committed_holdout_set_has_no_unexplained_mismatches(): + """Every entry's actual outcome must equal what it claims to expect -- + a mismatch anywhere here means either the fixture's own expectation is + wrong, or real behaviour drifted from it, and either way the published + counts above would be misleading without this check.""" + score = score_holdout(load_holdout()) + mismatches = [d for d in score["details"] if d["expected"] != d["actual"]] + assert mismatches == [] + + +def test_the_committed_holdout_set_pins_the_70_regression(): + """Q05 names the #70 bug by id in the fixture itself -- confirm it is + still there and still answered_correct, not quietly dropped.""" + entries = load_holdout() + q05 = next(e for e in entries if e["id"] == "Q05") + assert "10.20.20.0/24" in q05["question"] + score = score_holdout([q05]) + assert score["counts"]["answered_correct"] == 1 From b294fe417f5652847acfab1f43246d57ff8bef14 Mon Sep 17 00:00:00 2001 From: patelankeet2 Date: Wed, 21 Oct 2026 11:23:39 +1300 Subject: [PATCH 3/3] docs/evaluation.md: record that Part 2 is now also measured in CI (#393) Dated update note, not a rewrite -- the original 10 August measurement stays as the historical record, same convention this file already uses elsewhere (the #70 entry a few paragraphs above does the same). Points at tools/query_quality.py and names the one real difference from #346's held-out set: no model means no staleness risk, so there is no hash check here, by design, not by oversight. --- docs/evaluation.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/evaluation.md b/docs/evaluation.md index 4c02d0f..61beaf7 100644 --- a/docs/evaluation.md +++ b/docs/evaluation.md @@ -386,6 +386,18 @@ That is the sequence worth keeping: an evaluation found a defect in shipped code, the defect was fixed the same day, and the evaluation was re-run rather than edited to match. +> **UPDATE, #393 (US-60).** The measurement above was done once, by hand, on +> 10 August. `tools/query_quality.py` now runs an equivalent held-out +> question set in CI, every time -- 20 entries across all three intents, +> scored against the same four outcomes this section already uses, with the +> same clean record: 0 answered-wrong. Unlike `ai/explain.py`'s held-out set +> (#346), `ai/query.py` calls no model, so the tool executes the real +> `answer_question()` fresh on every run rather than replaying a frozen +> capture -- there is nothing here that can go stale the way captured model +> text can. The exact 20 questions measured on 10 August were never +> preserved verbatim, so this is a rebuilt, equivalent set rather than a +> literal replay; see `tests/fixtures/query_quality/holdout.json`. + --- ## Part 3, is the explanation text any good? (#90)