From c5b254c6652a60ed7d72692b601276ad5c914e07 Mon Sep 17 00:00:00 2001 From: abrichr Date: Thu, 3 Sep 2026 17:21:15 -0400 Subject: [PATCH] fix(reward,extradup): resolve identity every call, and stop selling a 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 --- ..._CERTIFIED_REWARD_RL_PILOT_2026_09_02.json | 10 +- docs/reward/README.md | 40 +++- openadapt_evals/extradup/README.md | 19 +- openadapt_evals/extradup/checkers.py | 24 ++- openadapt_evals/reward/devsigner.py | 46 +++-- openadapt_evals/reward/proof.py | 9 +- openadapt_evals/reward/receipts.py | 27 +++ openadapt_evals/reward/trl.py | 9 +- openadapt_evals/reward/verl.py | 5 +- pyproject.toml | 8 +- tests/test_extradup_kit.py | 60 ++++++ tests/test_reward_certificate_narrowing.py | 171 ++++++++++++++++++ uv.lock | 2 +- 13 files changed, 393 insertions(+), 37 deletions(-) create mode 100644 tests/test_reward_certificate_narrowing.py diff --git a/docs/preregistrations/M_FREEZE_CERTIFIED_REWARD_RL_PILOT_2026_09_02.json b/docs/preregistrations/M_FREEZE_CERTIFIED_REWARD_RL_PILOT_2026_09_02.json index 0ba3985..1d59a51 100644 --- a/docs/preregistrations/M_FREEZE_CERTIFIED_REWARD_RL_PILOT_2026_09_02.json +++ b/docs/preregistrations/M_FREEZE_CERTIFIED_REWARD_RL_PILOT_2026_09_02.json @@ -320,13 +320,13 @@ }, { "path": "openadapt_evals/extradup/checkers.py", - "sha256": "sha256:e53bef06e540cd718c4b33699b379046b2d747c6d3146a8bf08ad7a8cf439e6c", - "git_blob": "9bd854090a257d9f4449f48a470536aebe9859ac" + "sha256": "sha256:a5c73116db2000aa17e46abae1616cca0c694f82742ace52bad5a10888b34f71", + "git_blob": "e2669ff809be7add2a99d1174e6760cd37223c3c" }, { "path": "openadapt_evals/reward/proof.py", - "sha256": "sha256:7f87d60b9250110c5f10bad3e4b4f0f80209711b211d9ed41ed1d2cd2bff4466", - "git_blob": "20da8e6168ad03220aa1a21498d40a0c5f9f7626" + "sha256": "sha256:f278a66356a8469d06772bc558f89e6a6f7fb6265db993b51f71acc51ea662a0", + "git_blob": "ecdff94d80440b40d19f0a0a4bf18a87d5490e71" }, { "path": "docs/reward/proof_2026-09-01.json", @@ -347,7 +347,7 @@ "proof_path": "docs/reward/proof_2026-09-02.json", "note": "Wrong-record write. Named identity is p1; write lands on p2. Not an ExtraDup operator." }, - "amendment_note": "2026-09-02 identity-swap: proof.py hash and proof_2026-09-02.json added. ExtraDup operators and the 2026-09-01 certificate digest are unchanged. 2026-09-02 wrong_record: mutations.py, gold.py and checkers.py hashes updated for the eval-only wrong_record family and the identity resolution inside sor_check. OPERATORS, MUTANTS, holdout_operators, the seed schedule, and the 2026-09-01 certificate digest are unchanged; wrong_record lives in EVAL_ONLY_OPERATORS and is not an ExtraDup operator.", + "amendment_note": "2026-09-02 identity-swap: proof.py hash and proof_2026-09-02.json added. ExtraDup operators and the 2026-09-01 certificate digest are unchanged. 2026-09-02 wrong_record: mutations.py, gold.py and checkers.py hashes updated for the eval-only wrong_record family and the identity resolution inside sor_check. OPERATORS, MUTANTS, holdout_operators, the seed schedule, and the 2026-09-01 certificate digest are unchanged; wrong_record lives in EVAL_ONLY_OPERATORS and is not an ExtraDup operator. 2026-09-03 unconditional identity resolution: checkers.py and proof.py hashes updated. sor_check resolved the record by oracle_identity only when nothing had landed there, so at |spec(M)| > 1 a write that split between the named record and another one returned PASS. The resolution now runs on every call and decides the verdict; the reason strings for the frozen families are unchanged, and proof_2026-09-01.json and proof_2026-09-02.json are byte-identical. proof.py drops the calibration_scope argument that no longer exists on DevelopmentSigner and passes the contract's certificate_policy to assess_receipt. OPERATORS, MUTANTS, holdout_operators, the seed schedule, the arms, and the 2026-09-01 certificate digest are unchanged. No result was reported against this freeze.", "wrong_record": { "in_extradup_mutants": false, "in_training_dataset": false, diff --git a/docs/reward/README.md b/docs/reward/README.md index 731f836..8da1ff7 100644 --- a/docs/reward/README.md +++ b/docs/reward/README.md @@ -24,15 +24,46 @@ synthetic scope, the same scope this page describes. ## What today's certificate is -There is exactly one certificate scope in use today: **synthetic**. It is -calibrated on the MockMed ExtraDup rollouts in this repository and it bounds -those rollouts and nothing else. A **production** scope needs the Phase-1 -calibration described in +There is exactly one certificate scope: **synthetic**. It is calibrated on the +MockMed ExtraDup rollouts in this repository and it bounds those rollouts and +nothing else. A **production** scope needs the Phase-1 calibration described in [`docs/preregistrations/PREREGISTRATION_CERTIFIED_REWARD_RL_2026_08_25.md`](../preregistrations/PREREGISTRATION_CERTIFIED_REWARD_RL_2026_08_25.md), which is not published. Nothing in this package labels a reward `certified` without a scope on the receipt, and it logs the scope beside every certified receipt so a training log cannot hide which one it trained on. +`DevelopmentSigner.issue_certificate` takes no scope and no issuer argument. +It always mints `calibration_scope: synthetic` and `issuer: self_signed`, +because a key derived from a seed can honestly claim one thing: someone +computed a bound on a synthetic corpus. Both used to be plain parameters, and +passing `issuer="organization"` produced a receipt reading `certified: true, +calibration_scope: production, production_certified: true` from this package +alone, with no worker, no oracle, and no read. + +## The contract's certificate policy is the bar + +Every `RewardContractV1` names the bound it demands in `certificate_policy`. +`issue_receipt` marks an episode `certified` only when the certificate names +that same contract by digest and clears the policy: epsilon and delta no +looser, the same threshold, the same calibration corpus, an expiry no longer. +A certificate measured at epsilon 0.248885 against a contract demanding 0.05 +scores its scalar and is not certified. Handing `issue_receipt` a certificate +for a different contract raises rather than downgrades, because that is a +wiring bug and not a weak bound. + +A trainer that holds the contract can make the same check on the way in. Pass +`certificate_policy=contract.certificate_policy` to `CertifiedRewardFunction`, +to the verl manager, or to `assess_receipt` directly, and a certificate weaker +than the contract asked for stops counting as certified, with the two bounds +logged side by side. Without it, a trainer that was handed only a contract +digest has nothing to compare against and the receipt's own flag stands. + +Two things this stack does not do, so nobody assumes otherwise. Nobody looks +up an issuer key: `verify_signature` checks a signature against a public key +you already hold, and there is no registry that says which keys count. And +there is no revocation list. A certificate stops being current when its +policy-update expiry runs out. + ## Wiring TRL TRL's `GRPOTrainer` accepts reward functions that take `prompts`, `completions`, @@ -54,6 +85,7 @@ reward = CertifiedRewardFunction( policy_checkpoint_id="policy.checkpoint.0001", num_generations=config.num_generations, # TRL's group size certificate=certificate, # RewardCertificateV1 the trainer holds + certificate_policy=contract.certificate_policy, # the bound the contract demands ) trainer = GRPOTrainer(model=model, args=config, reward_funcs=[reward.as_async()], ...) diff --git a/openadapt_evals/extradup/README.md b/openadapt_evals/extradup/README.md index 690146d..5721177 100644 --- a/openadapt_evals/extradup/README.md +++ b/openadapt_evals/extradup/README.md @@ -113,7 +113,7 @@ spec's `pubpid` on the OpenEMR-shaped store. `|new| = 1`, which matches gold. The banner says saved, same as gold. `content_only_check` PASSes, because the content is right. `identity_check` resolves the row set under the contract's `oracle_identity`, finds nothing, and FAILs. `sor_check` runs that -resolution first and reports which record the write did land on. +resolution on every call and reports which record the write did land on. Worth knowing before you copy this into a real oracle: MockMed carries `patient_id` as a typed form field, so a plain field comparison happens to @@ -126,6 +126,23 @@ works when the identity isn't in the payload. `control`. The gold write. SoR PASS, Seal `VERIFIED`. That's how we know the oracle isn't stuck on FAIL. +## Two counts, both asked every time + +`sor_check` asks `|new(M)| = |spec(M)|`, so nothing landed anywhere it should +not, and `|new(M) under oracle_identity| = |spec(M)|`, so the right number +landed on the record the contract named. Only one of the two moves when a +write splits between the named record and another one. + +Both gold specs write one record. At `|spec(M)| = 1` a right count forces a +right subject, so the two questions are hard to tell apart here, and an +earlier version resolved identity only when nothing at all had landed under +the contract identity. That guard was invisible to every shipped fixture and +wrong above 1. Set `expected_new = 2`, add one correct row and one on the +decoy patient, and it returned PASS while `identity_check` returned FAIL. + +If you write your own spec, this is the line to keep. The count you can read +off a screenshot is not the count that says which chart you wrote to. + ## What this kit does not ship It doesn't vendor AppWorld `evaluation.py`. HOLDOUT stays sealed. WorkArena diff --git a/openadapt_evals/extradup/checkers.py b/openadapt_evals/extradup/checkers.py index 9bd8540..e2669ff 100644 --- a/openadapt_evals/extradup/checkers.py +++ b/openadapt_evals/extradup/checkers.py @@ -97,17 +97,33 @@ def sor_check( patient dies on the identity resolution, not on a field comparison: the identity keys are checked as identity, so removing one from the content loop cannot silently reopen the wrong-record hole. + + The verdict asks both counts, always. ``|new(M)| = |spec(M)|`` says + nothing landed anywhere it should not. ``|new(M) under oracle_identity| = + |spec(M)|`` says the right number landed on the subject the contract + named. Only one of the two moves when a write splits between the right + record and another one. + + An earlier version resolved identity only when nothing at all had landed + under the contract identity, which made the WHERE question invisible + whenever the count happened to agree. Both gold specs write one record, + and at ``|spec(M)| = 1`` a right count forces a right subject, so no + shipped test could reach it. At ``expected_new=2``, one correct row plus + one on the decoy patient returned PASS. """ added = new_records(before, after) + landed = records_under(identity_of(spec), added) reasons: list[str] = [] if len(added) != spec.expected_new: reasons.append( f"|new(M)|={len(added)} != |spec(M)|={spec.expected_new}" ) - # Only when the write landed somewhere but nowhere under the contract - # identity. When some record did land there, cardinality already speaks - # and a second count would say nothing new. - if added and not records_under(identity_of(spec), added): + # This condition governs the wording, never the verdict. When every added + # row is already under the contract identity the two counts are the same + # number about the same rows, and the line above has said it. When they + # differ, the identity reason carries what the cardinality line cannot: + # which other subject the write reached. + if len(landed) != spec.expected_new and len(landed) != len(added): reasons.extend(identity_check(spec, before, after).reasons) content = { key: value for key, value in spec.fields.items() if key not in spec.identity_fields diff --git a/openadapt_evals/reward/devsigner.py b/openadapt_evals/reward/devsigner.py index 3c369de..b5d05ad 100644 --- a/openadapt_evals/reward/devsigner.py +++ b/openadapt_evals/reward/devsigner.py @@ -81,13 +81,20 @@ def issue_certificate( issued_at_policy_update: int, issued_at: str, expiry_policy_updates: int | None = None, - calibration_scope: str = SYNTHETIC_SCOPE, - issuer: str = SELF_SIGNED, ) -> RewardCertificateV1: - """Issue a certificate that satisfies ``contract.certificate_policy``. - - The contract allows a self-signed certificate to carry synthetic - scope only; passing another scope raises at validation. + """Issue a self-signed, synthetic-scope certificate for ``contract``. + + The scope and the issuer are not parameters. This signer holds a key + derived from a seed, and nobody verifies that key, so a certificate + from here can honestly claim one thing: someone computed a bound on a + synthetic corpus. Passing ``issuer="organization"`` used to buy a + receipt reading ``certified: true, calibration_scope: production`` + with no worker, no oracle, and no read. There is no parameter to pass + now, and ``openadapt-types`` has no enum member to accept. + + Every field except the identifiers is copied from + ``contract.certificate_policy``, so the certificate satisfies the + contract that asked for it. """ policy = contract.certificate_policy @@ -102,8 +109,8 @@ def issue_certificate( "issued_at_policy_update": issued_at_policy_update, "expiry_policy_updates": expiry_policy_updates or policy.expiry_policy_updates, "issued_at": issued_at, - "calibration_scope": calibration_scope, - "issuer": issuer, + "calibration_scope": SYNTHETIC_SCOPE, + "issuer": SELF_SIGNED, "issuer_key_id": self.key_id, } unsigned = RewardCertificateV1.model_validate({**payload, "signature": "A" * 86 + "=="}) @@ -129,21 +136,34 @@ def issue_receipt( ) -> RewardEvidenceReceiptV1: """Issue a receipt whose flags follow the contract's own rules. - ``certified`` is true only at tier 2 or 3 with a certificate current - at ``policy_update``. The calibration scope and corpus digest come - from that certificate; a receipt with no certificate carries neither. - Unscored outcomes carry no scalar and no components. The caller - cannot override any of this. + ``certified`` is true only at tier 2 or 3 with a certificate that is + current at ``policy_update``, names this contract by digest, and + clears ``contract.certificate_policy``. That last check is the one + that was missing: a certificate whose measured epsilon was 0.248885 + against a contract demanding 0.05 produced ``certified`` anyway. + + The calibration scope and corpus digest come from that certificate; a + receipt with no certificate carries neither. Unscored outcomes carry + no scalar and no components. The caller cannot override any of this. """ outcome = RewardOutcomeV1(outcome) tier = int(oracle_tier) development_only = tier < REWARD_CERTIFIED_MINIMUM_TIER state = certificate_state(certificate, policy_update) + if ( + certificate is not None + and certificate.reward_contract_digest != contract.digest + ): + raise ValueError( + f"certificate {certificate.certificate_id} names reward contract " + f"{certificate.reward_contract_digest}, not {contract.digest}" + ) certified = ( not development_only and certificate is not None and state is RewardCertificateStateV1.CURRENT + and certificate.satisfies(contract.certificate_policy) ) scalar = scoring.scalar_for(outcome) unscored = scalar is None diff --git a/openadapt_evals/reward/proof.py b/openadapt_evals/reward/proof.py index 20da8e6..ecdff94 100644 --- a/openadapt_evals/reward/proof.py +++ b/openadapt_evals/reward/proof.py @@ -62,11 +62,7 @@ from openadapt_evals.extradup.gold import MOCKMED_GOLD from openadapt_evals.extradup.mutations import OPERATORS, apply from openadapt_evals.extradup.store import MockMedStore, Screen -from openadapt_evals.reward.devsigner import ( - SYNTHETIC_SCOPE, - DevelopmentSigner, - sha256_digest, -) +from openadapt_evals.reward.devsigner import DevelopmentSigner, sha256_digest from openadapt_evals.reward.receipts import ScoredEpisode, assess_receipt logger = logging.getLogger(__name__) @@ -447,6 +443,7 @@ def expiry_check( policy_update=update, expected_contract_digest=contract.digest, certificate=certificate, + certificate_policy=contract.certificate_policy, ) checked += 1 still_certified += int(episode.certified) @@ -482,7 +479,6 @@ def run_proof( ), issued_at_policy_update=0, issued_at=ISSUED_AT, - calibration_scope=SYNTHETIC_SCOPE, ) receipts = issue_receipts(signer, contract, certificate, rollouts) scored: dict[tuple[str, int, str], ScoredEpisode] = {} @@ -493,6 +489,7 @@ def run_proof( expected_contract_digest=contract.digest, expected_episode_id=receipt.episode_id, certificate=certificate if key[2] == CERTIFIED else None, + certificate_policy=contract.certificate_policy, ) run = ProofRun( seeds=seeds, diff --git a/openadapt_evals/reward/receipts.py b/openadapt_evals/reward/receipts.py index 3b601e7..0c3a380 100644 --- a/openadapt_evals/reward/receipts.py +++ b/openadapt_evals/reward/receipts.py @@ -20,6 +20,7 @@ from openadapt_types.reward import ( REWARD_CERTIFIED_MINIMUM_TIER, + RewardCertificatePolicyV1, RewardCertificateStateV1, RewardCertificateV1, RewardCertificationRefused, @@ -356,10 +357,17 @@ def assess_receipt( expected_contract_digest: str | None = None, expected_episode_id: str | None = None, certificate: RewardCertificateV1 | None = None, + certificate_policy: RewardCertificatePolicyV1 | None = None, ) -> ScoredEpisode: """Turn one receipt into a scored episode, applying the trainer-side rules. * The receipt must bind the expected contract digest and episode id. + * When the trainer holds both the certificate and the contract's + ``certificate_policy``, the certificate has to clear that policy. A + certificate with a looser epsilon, delta, threshold, corpus, or expiry + than the contract asked for turns ``certified`` off here, and says so + at WARNING. Omitting the policy leaves the receipt's own flag standing, + which is what a trainer that was handed only a contract digest can do. * ``development_only`` (tier 0 or 1) can never be certified. The receipt contract already forbids that combination; this re-checks it so a receipt built outside the contract cannot slip through. @@ -414,6 +422,25 @@ def assess_receipt( verdict = score(receipt.reward_outcome, tier, certificate, policy_update) state = certificate.state_at(policy_update) certified = bool(receipt.certified and verdict.certified) + if certificate_policy is not None and not certificate.satisfies( + certificate_policy + ): + logger.warning( + "reward certificate %s is weaker than the contract policy " + "(epsilon=%s delta=%s threshold=%s expiry=%d against epsilon=%s " + "delta=%s threshold=%s expiry=%d); receipt %s is not certified", + receipt.certificate_id, + certificate.epsilon, + certificate.delta, + certificate.threshold, + certificate.expiry_policy_updates, + certificate_policy.epsilon, + certificate_policy.delta, + certificate_policy.threshold, + certificate_policy.expiry_policy_updates, + receipt.receipt_id, + ) + certified = False else: state = receipt.certificate_state certified = bool( diff --git a/openadapt_evals/reward/trl.py b/openadapt_evals/reward/trl.py index 4ac0fee..93473a8 100644 --- a/openadapt_evals/reward/trl.py +++ b/openadapt_evals/reward/trl.py @@ -32,7 +32,7 @@ from collections.abc import Callable, Sequence from typing import Any -from openadapt_types.reward import RewardCertificateV1 +from openadapt_types.reward import RewardCertificatePolicyV1, RewardCertificateV1 from openadapt_evals.reward.receipts import ( ORACLE_IDENTITY_KEY, @@ -76,6 +76,10 @@ class CertifiedRewardFunction: receipt is then logged as ``development_only``. certificate: the certificate the trainer holds. When given, expiry is re-evaluated at the trainer's own global step. + certificate_policy: the contract's ``certificate_policy``. Give it + alongside ``certificate`` and the trainer checks that the bound + it holds is at least as tight as the contract demanded; a looser + certificate stops being counted as certified. policy_update: overrides ``trainer_state.global_step`` (an int or a zero-argument callable). episode_id_column: the dataset column that names each episode. @@ -107,6 +111,7 @@ def __init__( num_generations: int | None = None, require_certified: bool = True, certificate: RewardCertificateV1 | None = None, + certificate_policy: RewardCertificatePolicyV1 | None = None, policy_update: int | Callable[[], int] | None = None, episode_id_column: str = "episode_id", task_id_column: str | None = "task_id", @@ -123,6 +128,7 @@ def __init__( self.num_generations = num_generations self.require_certified = require_certified self.certificate = certificate + self.certificate_policy = certificate_policy self._policy_update = policy_update self.episode_id_column = episode_id_column self.task_id_column = task_id_column @@ -247,6 +253,7 @@ def _assess( expected_contract_digest=self.reward_contract_digest, expected_episode_id=descriptor.episode_id, certificate=self.certificate, + certificate_policy=self.certificate_policy, ) if self.require_certified: require_certified_or_unscored(episode) diff --git a/openadapt_evals/reward/verl.py b/openadapt_evals/reward/verl.py index dff0898..019e2f2 100644 --- a/openadapt_evals/reward/verl.py +++ b/openadapt_evals/reward/verl.py @@ -33,7 +33,7 @@ from collections.abc import Callable, Sequence from typing import Any -from openadapt_types.reward import RewardCertificateV1 +from openadapt_types.reward import RewardCertificatePolicyV1, RewardCertificateV1 from openadapt_evals.reward.receipts import ( ORACLE_IDENTITY_KEY, @@ -107,6 +107,7 @@ def __init__( policy_checkpoint_id: str, require_certified: bool = True, certificate: RewardCertificateV1 | None = None, + certificate_policy: RewardCertificatePolicyV1 | None = None, policy_update: int | Callable[[], int] | None = None, episode_id_key: str = "episode_id", task_id_key: str | None = "task_id", @@ -130,6 +131,7 @@ def __init__( self.policy_checkpoint_id = policy_checkpoint_id self.require_certified = require_certified self.certificate = certificate + self.certificate_policy = certificate_policy self._policy_update = policy_update self.episode_id_key = episode_id_key self.task_id_key = task_id_key @@ -209,6 +211,7 @@ def score_batch( expected_contract_digest=self.reward_contract_digest, expected_episode_id=descriptor.episode_id, certificate=self.certificate, + certificate_policy=self.certificate_policy, ) if self.require_certified: require_certified_or_unscored(episode) diff --git a/pyproject.toml b/pyproject.toml index 286820a..747f7d3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,7 +47,13 @@ dependencies = [ "openadapt-consilium>=0.3.2", "openadapt-telemetry>=0.2.0", # Requires openadapt_types.reward (RewardEvidenceReceiptV1, RewardCertificateV1, score). - "openadapt-types>=0.17.0", + # Bounded pin, same as openadapt-flow uses. openadapt-types is 0-based semver + # with major_on_zero=false, so a breaking change lands as a minor bump and an + # unbounded floor would take it silently. openadapt-types#40 narrows the + # certificate contracts and changes score() to require the reward contract; + # widening this bound and updating assess_receipt go in one PR after that + # release, not before. + "openadapt-types>=0.17.0,<0.18.0", ] [project.optional-dependencies] diff --git a/tests/test_extradup_kit.py b/tests/test_extradup_kit.py index f89502a..84db84d 100644 --- a/tests/test_extradup_kit.py +++ b/tests/test_extradup_kit.py @@ -2,6 +2,9 @@ from __future__ import annotations +from dataclasses import replace +from itertools import product + import pytest from openadapt_evals.extradup import ( @@ -18,6 +21,7 @@ field_inclusion_check, identity_check, new_records, + records_under, sor_check, visual_only_check, ) @@ -173,3 +177,59 @@ def test_python_module_check() -> None: assert main(["list"]) == 0 assert main(["run", "mockmed:dup"]) == 0 assert main(["run", "mockmed:control"]) == 0 + + +def test_identity_is_resolved_even_when_the_count_agrees() -> None: + """`expected_new=2`, one correct row and one on the decoy patient. + + Guarding the identity resolution on "nothing landed under the contract + identity" hid this: `sor_check` returned PASS while `identity_check` + returned FAIL and named the decoy. Both gold specs write one record, + where a right count forces a right subject, so no shipped fixture could + reach it. + """ + spec = replace(MOCKMED_GOLD, expected_new=2) + identity = identity_of(spec) + decoy = decoy_of(spec) + before = [{"id": "e0", **identity, "type": "Triage", "note": "old"}] + content = {key: value for key, value in spec.fields.items() if key not in identity} + correct = {"id": "e1", **identity, **content} + elsewhere = {"id": "e2", **decoy, **content} + + split = sor_check(spec, before, before + [correct, elsewhere]) + assert split.verdict == "FAIL" + assert identity_check(spec, before, before + [correct, elsewhere]).verdict == "FAIL" + reasons = ";".join(split.reasons) + assert "under oracle_identity" in reasons + assert str(decoy) in reasons + + both_right = {"id": "e3", **identity, **content} + assert sor_check(spec, before, before + [correct, both_right]).verdict == "PASS" + + +def test_the_wording_rule_never_decides_the_verdict() -> None: + """`sor_check` suppresses a redundant sentence, never a check. + + The identity reason 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. Whenever identity resolution fails, + one of the two lines fires. + """ + spec = replace(MOCKMED_GOLD, expected_new=2) + identity = identity_of(spec) + decoy = decoy_of(spec) + content = {key: value for key, value in spec.fields.items() if key not in identity} + rows = { + "right": {**identity, **content}, + "wrong": {**decoy, **content}, + } + for count in range(4): + for places in product(("right", "wrong"), repeat=count): + after = [ + {"id": f"e{index}", **rows[place]} for index, place in enumerate(places) + ] + landed = len(records_under(identity, after)) + result = sor_check(spec, [], after) + assert result.ok == ( + landed == spec.expected_new and len(after) == spec.expected_new + ), (places, result) diff --git a/tests/test_reward_certificate_narrowing.py b/tests/test_reward_certificate_narrowing.py new file mode 100644 index 0000000..27017b9 --- /dev/null +++ b/tests/test_reward_certificate_narrowing.py @@ -0,0 +1,171 @@ +"""A certificate cannot buy a claim nobody checks. + +Each test here drives a reproduction that succeeded against published +openadapt-evals 0.97.0. +""" + +from __future__ import annotations + +import logging +from typing import Any + +import pytest +from openadapt_types.reward import ( + RewardCalibrationScopeV1, + RewardCertificateIssuerV1, + RewardCertificateV1, + RewardOutcomeV1, +) + +from openadapt_evals.reward.devsigner import ( + SELF_SIGNED, + SYNTHETIC_SCOPE, + sha256_digest, + verify_signature, +) +from openadapt_evals.reward.receipts import assess_receipt +from tests.reward_fixtures import CONTRACT, ISSUED_AT, POLICY_CHECKPOINT, SIGNER + + +def _certificate(**overrides: Any) -> RewardCertificateV1: + """Mint a certificate with fields the signer would never choose itself.""" + + policy = CONTRACT.certificate_policy + payload: dict[str, Any] = { + "certificate_id": "reward.certificate.narrowing-0001", + "reward_contract_digest": CONTRACT.digest, + "checker_configuration_digest": sha256_digest(b"checker"), + "epsilon": policy.epsilon, + "delta": policy.delta, + "threshold": policy.threshold, + "calibration_corpus_digest": policy.calibration_corpus_digest, + "calibration_scope": SYNTHETIC_SCOPE, + "issued_at_policy_update": 0, + "expiry_policy_updates": policy.expiry_policy_updates, + "issued_at": ISSUED_AT, + "issuer": SELF_SIGNED, + "issuer_key_id": SIGNER.key_id, + } + payload.update(overrides) + unsigned = RewardCertificateV1.model_validate({**payload, "signature": "A" * 86 + "=="}) + payload["signature"] = SIGNER.sign(unsigned.unsigned_payload()) + return RewardCertificateV1.model_validate(payload) + + +def _receipt(certificate: RewardCertificateV1 | None, **kwargs: Any): + return SIGNER.issue_receipt( + contract=kwargs.pop("contract", CONTRACT), + receipt_id="receipt.narrowing-0001", + episode_id="episode.narrowing-0001", + policy_checkpoint_id=POLICY_CHECKPOINT, + policy_update=1, + oracle_tier=2, + outcome=RewardOutcomeV1.VERIFIED, + evidence_digest=sha256_digest(b"evidence"), + nonce="nonce.narrowing-0001", + issued_at=ISSUED_AT, + certificate=certificate, + **kwargs, + ) + + +def _issue(**kwargs: Any) -> RewardCertificateV1: + return SIGNER.issue_certificate( + CONTRACT, + certificate_id="reward.certificate.narrowing-0002", + checker_configuration_digest=sha256_digest(b"checker"), + issued_at_policy_update=0, + issued_at=ISSUED_AT, + **kwargs, + ) + + +def test_the_signer_takes_no_issuer_or_scope_argument() -> None: + """`issuer="organization"` bought `certified: true, scope: production`.""" + + with pytest.raises(TypeError, match="issuer"): + _issue(issuer="organization") + with pytest.raises(TypeError, match="calibration_scope"): + _issue(calibration_scope="production") + + certificate = _issue() + assert certificate.issuer is RewardCertificateIssuerV1.SELF_SIGNED + assert certificate.calibration_scope is RewardCalibrationScopeV1.SYNTHETIC + assert verify_signature(certificate, SIGNER.public_key_bytes()) + + +def test_a_receipt_never_states_a_production_scope() -> None: + receipt = _receipt(_issue()) + assert receipt.certified is True + assert receipt.calibration_scope is RewardCalibrationScopeV1.SYNTHETIC + assert receipt.model_dump(mode="json")["calibration_scope"] == "synthetic" + + +def test_a_certificate_weaker_than_the_contract_is_not_certified() -> None: + """Measured epsilon 0.248885 against a contract demanding 0.05.""" + + weak = _certificate(epsilon=0.248885) + assert CONTRACT.certificate_policy.epsilon == 0.05 + assert weak.satisfies(CONTRACT.certificate_policy) is False + + receipt = _receipt(weak) + assert receipt.certified is False + assert receipt.scalar_reward == 1.0 + + +@pytest.mark.parametrize( + ("field", "value"), + [ + ("delta", 0.9), + ("threshold", 0.25), + ("calibration_corpus_digest", sha256_digest(b"another corpus")), + ("expiry_policy_updates", 11), + ], +) +def test_every_shortfall_stops_certification(field: str, value: Any) -> None: + receipt = _receipt(_certificate(**{field: value})) + assert receipt.certified is False + + +def test_a_certificate_for_another_contract_is_refused() -> None: + other = CONTRACT.model_copy(update={"task_id": "task.test.0002"}) + with pytest.raises(ValueError, match="names reward contract"): + _receipt(_issue(), contract=other) + + +def test_the_trainer_refuses_a_weak_certificate_it_can_measure( + caplog: pytest.LogCaptureFixture, +) -> None: + """`assess_receipt` given the contract policy rechecks the bound itself.""" + + weak = _certificate(epsilon=0.248885) + # A receipt the worker built before this refusal existed. + payload = _receipt(_issue()).model_dump(mode="json") + payload["certificate_id"] = weak.certificate_id + payload["certificate_digest"] = weak.digest + unsigned = type(_receipt(_issue())).model_validate( + {**payload, "signature": "A" * 86 + "=="} + ) + payload["signature"] = SIGNER.sign(unsigned.unsigned_payload()) + stale = type(unsigned).model_validate(payload) + assert stale.certified is True + + trusting = assess_receipt( + stale, + policy_update=1, + expected_contract_digest=CONTRACT.digest, + certificate=weak, + ) + assert trusting.certified is True + + with caplog.at_level(logging.WARNING, logger="openadapt_evals.reward.receipts"): + checked = assess_receipt( + stale, + policy_update=1, + expected_contract_digest=CONTRACT.digest, + certificate=weak, + certificate_policy=CONTRACT.certificate_policy, + ) + assert checked.certified is False + assert checked.scalar == 1.0 + assert "weaker than the contract policy" in caplog.text diff --git a/uv.lock b/uv.lock index e187c1a..253be64 100644 --- a/uv.lock +++ b/uv.lock @@ -3614,7 +3614,7 @@ requires-dist = [ { name = "openadapt-ml", extras = ["training"], marker = "extra == 'training'", specifier = ">=0.11.0" }, { name = "openadapt-retrieval", marker = "extra == 'retrieval'", specifier = ">=0.1.0" }, { name = "openadapt-telemetry", specifier = ">=0.2.0" }, - { name = "openadapt-types", specifier = ">=0.17.0" }, + { name = "openadapt-types", specifier = ">=0.17.0,<0.18.0" }, { name = "openai", specifier = ">=1.0.0" }, { name = "outlines", marker = "extra == 'dev'", specifier = ">=0.1.0" }, { name = "outlines", marker = "extra == 'training'", specifier = ">=0.1.0" },