Repository navigation
A repeatable score for the question layer, run in CI (#393) - #407
Open
patelankeet2 wants to merge 3 commits into
Open
patelankeet2 wants to merge 3 commits into
patelankeet2 wants to merge 3 commits into
Conversation
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
…us 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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #393 (US-60).
What it does
A held-out question set for
ai/query.py, scored every time it runs, mirroringdocs/evaluation.mdPart 2's own four-outcome shape from the one hand measurement that has existed since 10 August (answered-correct, refused-correctly, refused-but-should-answer, and the dangerous one, answered-wrong).The one design difference from #385's held-out set, and why
#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.pycalls no model in either direction (CLAUDE.md7c) -- it is pure, deterministic classification and resolution over a snapshot. There is nothing here that can go stale the way #385's captured text can, so:FakeSessionconfiguration, the exact harnesstests/test_query_translation.pyalready uses) and the expected outcome.answer_question()against every entry, every single run -- in a unit test, in CI, by hand, two years from now. Stronger evidence than replaying a snapshot of a past run, not weaker.The held-out set
20 entries (
tests/fixtures/query_quality/holdout.json), 10 answered-correct / 10 refused-correctly, built from scenarios already individually proven intests/test_query_translation.pyrather than invented fresh -- plus Q05, named explicitly as a regression pin for the #70 bug (a network destination resolved to a real host, not the network address, which the historical measurement found and fixed the same day). The exact 20 questions measured on 10 August were never preserved verbatim indocs/evaluation.md, so this is a rebuilt, equivalent set rather than a literal replay -- said plainly in the doc update.A real gap I found in my own first version
While writing the classification logic, caught a case where an entry with no
answer_contains/answer_starts_withto check against would silently fall through toanswered_correctif the code answered when it should have refused -- nothing to text-match against means the check just... passed. Fixed before it ever shipped:should_groundis now compared explicitly against the entry's ownoutcome_expected, so "answered when a refusal was expected" is alwaysanswered_wrong, regardless of what the answer text happens to say. Pinned with its own test (test_answering_when_a_refusal_was_expected_is_answered_wrong_regardless_of_text) and mutation-tested by reverting to the original logic -- failed for exactly the right reason, reverted.Verified
Mutation-tested twice: flipped the committed fixture's own expectation on one entry (caught by both the threshold test and the mismatch test, reverted), and reverted the classification-logic fix described above (caught by its own dedicated test, reverted).
Acceptance criteria
ai/query.py, scored with the same method as A held-out, repeatable score for the explanation layer (#346) #385 (the pattern -- A held-out, repeatable score for the explanation layer (#346) #385 itself isn't merged yet; see note below)FakeSessionhere needs neitherdocs/evaluation.mdPart 2 doesOne thing worth knowing before merge
#393 says "depends on #385: its method is reused, so it merges first." #385 isn't merged -- its last
CHANGES_REQUESTEDreview is stale under M-2 (flagged separately, on #385 itself). This PR only reuses #385's pattern (the held-out-set-plus-documented-threshold shape), not its code or data, so it does not need #385 to land first -- #394 (US-61) is the one that actually needs #385's real numbers. Flagging this so nobody assumes the dependency note above means this PR is blocked when it isn't.