Skip to content

Catch a containment check changed without VERSION being bumped - #57

Merged
deepskandpal merged 2 commits into
deepskandpal:mainfrom
dchaudhari7177:32-containment-version-digest
Aug 30, 2026
Merged

Catch a containment check changed without VERSION being bumped#57
deepskandpal merged 2 commits into
deepskandpal:mainfrom
dchaudhari7177:32-containment-version-digest

Conversation

@dchaudhari7177

Copy link
Copy Markdown
Contributor

Closes #32

CHECKS_DIGEST beside VERSION in providers.py, and a test that fails when they drift apart.

Both directions verified against the real file

Fixtures prove the normaliser; these prove the guard:

$ uv run ruff format src/langchef/judge/providers.py
1 file left unchanged
# digest: fc7b2758193b -> fc7b2758193b   UNCHANGED

$ # add one word to HEDGE_KEYS
# digest: fc7b2758193b -> 4dcc398bc200   CHANGED

and the failure a developer actually sees:

AssertionError: The containment scoring checks changed but VERSION did not.

  VERSION is currently 'containment/v2', and it is part of the
  cache key. Leaving it alone means every workspace with a warm cache
  keeps serving judgements produced by the OLD check -- silently, and
  the numbers look exactly like the right numbers.

  If you changed a check on purpose:
    1. bump VERSION in src/langchef/judge/providers.py
    2. set CHECKS_DIGEST = '4dcc398bc200'

  If you only reformatted or edited a comment, this should not have
  fired -- the digest ignores both. Please say so on the issue, since
  a test that cries wolf is a test that gets deleted.

  recorded fc7b2758193b
  computed 4dcc398bc200

The digest covers the matched data, not just the code

HEDGES, HEDGE_KEYS, GROUNDED_KEYS, CORRECT_KEYS, WORD and STOPWORDS are all matched at judging time. Adding one word to HEDGE_KEYS changes which criterion a failure is cited under just as surely as editing the branch that reads it — the polarity inversion you describe in the issue is exactly this shape.

A digest over code alone would miss it, so a parametrised test monkeypatches each of the five in turn and asserts the digest moves. That keeps the coverage claim from rotting as the keywords change.

Why ruff format cannot trip it

The AC is explicit that reformatting must not fire, and the reason is in the issue: a test that cries wolf gets suppressed, and the real guarantee goes with it.

Normalisation tokenises the source and drops COMMENT and NL — precisely what ruff format emits when it reflows a call across lines, and what an edited comment produces.

Two deliberate non-drops:

  • INDENT / DEDENT are kept. Moving a statement into or out of an if is a behaviour change, not layout. A test asserts x = 1 at function level differs from the same line inside if a:.
  • String literals compare by value. ruff format normalises quote style, so 'x' and "x" must hash the same — but editing a rationale string is user-visible behaviour and must not. Both are tested.

Reuses the experiments.py shape

Same sha256(...).hexdigest()[:12], same idea: hash the content, record the hash next to the thing it authorises, and let a mismatch revoke it. experiments.py excludes [approval] from its own hash; here the digest covers named objects rather than the whole module, so CHECKS_DIGEST cannot hash itself.

Where the next person will look

Beside VERSION, in a comment that says what goes wrong, and the two edits to make. TRACKER.md's accepted-limit entry is rewritten from "nothing catches forgetting" to what now does.

One limit, stated rather than hidden

Adding or removing a magic trailing comma does move the digest. ruff format will not do that to already-formatted source — it only explodes a call when you add the comma yourself — so it is reachable by hand-editing only, and the message tells such a reader to report it. I'd rather leave that visible than special-case commas in the normaliser.

Two things I said on the issue I would confirm

  1. Where the recorded digest lives. Beside VERSION in providers.py. It matches experiments.py's pattern of recording the hash next to what it authorises, and keeps the two things that must move together adjacent. Happy to move it into the test if you'd rather keep generated constants out of the source.
  2. That the demonstrations are real. Both runs above are on the actual file, not fixtures.

Verification

uv run ruff check .          # All checks passed!
uv run ruff format --check . # 82 files already formatted
uv run pytest tests/         # 193 passed, 1 failed

Collected tests go 183 → 194 (+11). The single failure is test_docs_in_sync.py::test_verify_script_is_executable, which fails identically on a clean main on this machine — Git for Windows does not preserve the executable bit on scripts/verify.sh. It is unrelated to this branch and should be green on Linux CI.

VERSION is part of the cache key, so editing a scoring check without bumping it
leaves every workspace with a warm cache serving judgements from the old check.
Nothing errors and nothing warns; the numbers are simply wrong and they look
exactly like the right numbers. TRACKER.md has carried this as an accepted
limit, which was reasonable while the judge was new and is not reasonable now
that other people can clone the repository.

CHECKS_DIGEST records a content hash over everything a containment verdict
depends on, and a test fails when the two drift apart. It sits immediately
beside VERSION, so the two things that must move together are adjacent, and
carries the instructions in a comment where the next person will be editing.

The digest covers the matched data as well as the code. HEDGES, HEDGE_KEYS,
GROUNDED_KEYS, CORRECT_KEYS, WORD and STOPWORDS are all matched at judging
time -- adding one word to HEDGE_KEYS changes which criterion a failure is
cited under just as surely as editing the branch that reads it, and a digest
over code alone would not notice. A parametrised test asserts each of those
five is really covered, so the claim cannot rot.

Reformatting must not trip it, or the test gets suppressed and the real
guarantee goes with it. Normalisation tokenises the source and drops COMMENT
and NL, which is exactly what `ruff format` produces when it reflows a call or
a developer edits a comment. INDENT and DEDENT are deliberately kept: moving a
statement into or out of an `if` is a behaviour change. String literals are
compared by value rather than by their quoting, since `ruff format` normalises
quotes.

Verified both directions against the real file rather than only against
fixtures: `ruff format src/langchef/judge/providers.py` leaves the digest at
fc7b2758193b, and adding one word to HEDGE_KEYS moves it to 4dcc398bc200 and
fails the test with the VERSION-bump instructions rather than a bare hash
mismatch.

The failure message names the current VERSION, says why the cache makes this
dangerous, gives the two edits to make, and tells a reader who only reformatted
to report it -- because a test that cries wolf is a test that gets deleted.

One known limit, stated rather than hidden: adding or removing a magic trailing
comma does move the digest. `ruff format` will not do that to already-formatted
source, so it can only be reached by editing the comma by hand.
@deepskandpal

Copy link
Copy Markdown
Owner

Reviewed, and this is the strongest PR on the repo so far. Two calls in it are better than the issue asked for.

Keeping INDENT/DEDENT in the token stream while dropping COMMENT and NL is the right line to draw: moving a statement into a branch is behaviour, not layout. And reducing string literals to their value via ast.literal_eval means a quote-style change from ruff format is invisible while an edit to a rationale still trips, which is correct because rationales are user-visible output. Hashing the keyword tuples alongside the code was your extension and it was needed.

The failure message is the best part. Naming VERSION, its current value, the digest to paste, and asking the reader to report a false positive because "a test that cries wolf is a test that gets deleted" is exactly right.

One gap worth closing before merge. A threshold change does not trip the digest:

digest before GROUNDED_THRESHOLD 0.75 -> 0.95:  fc7b2758193b
digest after:                                    fc7b2758193b
                                                 NOT CAUGHT

GROUNDED_THRESHOLD and CONTAINMENT_THRESHOLD are module constants referenced by name inside judge, so the source of judge is unchanged when their values change, and neither is in parts. That is the same silent-stale-cache failure this PR exists to prevent: move 0.75 to 0.85, every borderline verdict flips, VERSION stays put, warm caches keep serving the old answers.

Adding repr(GROUNDED_THRESHOLD) and repr(CONTAINMENT_THRESHOLD) to parts plus a test in the shape of your keyword one should do it. Worth a scan for any other by-name reference while you are in there.

Also: main now requires both matrix legs, verify (python 3.12) and verify (python 3.13). This branch predates the matrix so it only has 3.12 and shows as blocked. Merging main will pick up the second leg.

@deepskandpal
deepskandpal merged commit 545325b into deepskandpal:main Aug 30, 2026
2 checks passed
deepskandpal added a commit that referenced this pull request Aug 30, 2026
#57 and #67 landed on origin while #28, #31 and #54 were being merged locally.
tests/test_flow.py conflicted: #28 added a per-criterion attribution test and
#58 added four ambiguity-disclosure tests, and git tangled them because both
open with identical fixture setup. Resolved as the union of both, reconstructed
from each side's full version rather than hand-patched, so no assertion was
silently dropped in the middle of a hunk.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nothing catches a forgotten containment VERSION bump

2 participants