fix(reward,extradup): resolve identity every call, and stop selling a bound nobody checked - #342
Merged
Merged
Conversation
… bound nobody checked
Two shipped defects, both found by adversarial review, both reproduced
against published openadapt-evals 0.97.0.
## The certificate was trusted on its shape
`DevelopmentSigner.issue_certificate` took `issuer` and `calibration_scope`
as plain parameters. `issuer="organization"` bought a receipt reading
`certified: true, calibration_scope: production, production_certified: true`
from this package alone, with no worker, no oracle and no read. Both
parameters are gone. The signer holds a key derived from a seed and nobody
verifies that key, so its certificate can honestly claim one thing: someone
computed a bound on a synthetic corpus. It always mints `synthetic` and
`self_signed`.
`RewardCertificateV1.satisfies` had one caller in the whole workspace and it
was a test. Nothing compared a certificate against the contract's own
`certificate_policy`, so a certificate whose measured epsilon was 0.248885
against a contract demanding 0.05 still produced `certified`. Now
`issue_receipt` certifies only a certificate that names this contract by
digest and clears its policy, and it raises on a certificate issued for a
different contract, because that is a wiring bug rather than a weak bound.
`assess_receipt` takes an optional `certificate_policy` and refuses
certification when the certificate it holds is weaker, logging both bounds
side by side. `CertifiedRewardFunction` and the verl manager pass it
through; the proof harness supplies `contract.certificate_policy`. A trainer
that was handed only a contract digest has nothing to compare against, so
omitting it leaves the receipt's own flag standing.
The types pin gains an upper bound, `<0.18.0`, the same one openadapt-flow
uses. openadapt-types is 0-based semver with `major_on_zero = false`, so a
breaking change arrives as a minor bump and an unbounded floor takes it
silently. openadapt-types#40 narrows these contracts and changes `score()`
to require the reward contract; widening this bound and updating
`assess_receipt` belong in one PR after that release.
## The identity check was guarded away
`sor_check` resolved the record by `oracle_identity` only when nothing at all
had landed there:
if added and not records_under(identity_of(spec), added):
so the WHERE question went unasked whenever cardinality happened to agree.
Both gold specs write one record, and at `|spec(M)| = 1` a right count forces
a right subject, so no shipped fixture could reach it. At `expected_new=2`,
one correct row plus one on the decoy patient returned `sor_check` PASS while
`identity_check` returned FAIL and named the decoy.
The resolution now runs on every call and decides the verdict. The reason
strings do not move for the frozen families: the identity sentence is left
out when every added row is already under the contract identity, because the
cardinality line then reports the same number about the same rows, and that
condition can never suppress a FAIL. `test_the_wording_rule_never_decides_the_verdict`
enumerates every arrangement of up to three rows and pins that.
Both proof artifacts are byte-identical and the hub environment's behaviour
is unchanged: the same seven cases, the same reasons, the same 95% upper
bound of 0.004270473020189125. The M-freeze repins `checkers.py` and
`proof.py` and records the amendment; it reports no result.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Two shipped defects, both found by adversarial review. Every reproduction below was run against published
openadapt-evals0.97.0 withopenadapt-types0.17.0 in a clean venv, and every one of them succeeded.1. A certificate was trusted on its shape
DevelopmentSigner.issue_certificatetookissuerandcalibration_scopeas plain parameters, and the types validator refused exactly one pair,self_signed+production. So:No worker, no oracle, no read. Both parameters are gone. This signer holds a key derived from a seed and nobody verifies that key, so its certificate can honestly claim one thing: someone computed a bound on a synthetic corpus. It always mints
syntheticandself_signed.Second reproduction, same run:
RewardCertificateV1.satisfieshad one caller in the workspace and it was a test. A certificate whose measured epsilon was 0.248885, against a contract whosecertificate_policydemanded 0.05, producedcertifiedanyway. A certificate issued for a completely different contract did too.issue_receiptnow certifies only a certificate that names this contract by digest and clears itscertificate_policy. It raises on a certificate for another contract, because that is a wiring bug and not a weak bound.assess_receipttakes an optionalcertificate_policy. Give it one and a certificate weaker than the contract asked for stops counting as certified, with both bounds logged side by side.CertifiedRewardFunctionand the verl manager pass it through, and the proof harness suppliescontract.certificate_policy. A trainer handed only a contract digest has nothing to compare against, so omitting it leaves the receipt's own flag standing.The pin
openadapt-typesmoves to>=0.17.0,<0.18.0, the boundopenadapt-flowalready uses. It is 0-based semver withmajor_on_zero = false, so a breaking change arrives as a minor bump and an unbounded floor takes it silently. openadapt-types#40 narrows these contracts and changesscore()to require the reward contract; widening this bound and updatingassess_receiptbelong in one PR after that release, which is why nothing here depends on it.2. The identity check was guarded away
openadapt_evals/extradup/checkers.py,sor_check:Identity was resolved only when nothing at all had landed under the contract identity. Set
expected_new=2, add one correct row and one on the decoy patient, andsor_checkreturns PASS whileidentity_checkreturns FAIL and names the decoy. Both gold specs useexpected_new=1, where a right count forces a right subject, so no shipped fixture could reach it. That is the same fixture-shaped blindness that hid the wrong-record defect this guard was added to fix.The resolution now runs on every call and decides the verdict. The reason strings do not move for the frozen families: the identity sentence is left out when every added row is already under the contract identity, because the cardinality line then reports the same number about the same rows. That condition governs wording only and can never suppress a FAIL, and
test_the_wording_rule_never_decides_the_verdictenumerates every arrangement of up to three rows to keep it that way.What a reviewer should check
The hub environment is unchanged.
openadapt-mockmed-extradupis live and pinned toopenadapt-evals>=0.97.0. Its self-test, its pytest file, andcheck_fails_closed.py --num-examples 2all ran here against this branch: the same eight cases, the same per-case rewards, the same reasons, and the same 95% Clopper-Pearson upper bound of 0.004270473020189125. Nothing needs pushing to the hub.The M-freeze is amended, not quietly rehashed.
checkers.pyandproof.pyare pinned by hash inM_FREEZE_CERTIFIED_REWARD_RL_PILOT_2026_09_02.json. Both hashes move, both are repinned, andamendment_noterecords what changed and why, in the same form the 2026-09-02 amendment used.proof_2026-09-01.jsonandproof_2026-09-02.jsonare byte-identical,OPERATORS,MUTANTS, the seed schedule and the certificate digest are untouched, and the freeze still reports no result.Nine new tests, eight of which fail on 0.97.0.
tests/test_reward_certificate_narrowing.pyand two additions totests/test_extradup_kit.py. The ninth is a positive assertion that the scope stays synthetic, which held before and still holds.Opened by an agent session, not the founder.
🤖 Generated with Claude Code