fix(agt-adapter): appraisal.status is the verifier's, so default it to none - #336
Merged
Merged
Conversation
…o none build_trust_record() set appraisal.status to "affirming" on every record it produced, and __init__ had no parameter to change it. The field is verifier-owned: spec section 3.3.1 says a verifier MUST record the depth it actually checked and MUST set the status to contraindicated when evidence fails, and models.Appraisal carries the same point in a comment, "What this verifier ran, not what the issuer claimed." The adapter set it at record-construction time, before signing and before any verifier existed. The result signed and verified, so a consumer reading the field to find out whether anybody checked was told yes by a record nobody had appraised. TraceSandboxAdapter already had this right, with appraisal_status defaulting to "none" and tests pinning both the default and the override. TraceAGTAdapter now matches it, and that was the only remaining hardcoded status in the package. This changes the content of records this adapter emits: an unappraised record now says none where it used to say affirming. A caller who really did appraise declares it with appraisal_status="affirming". Two documents described the old behaviour. docs/integration/agt.md said the adapter "populates an `affirming` appraisal without independently evaluating the session", and docs/tutorials/agt-adapter.md carried a post-hoc record["appraisal"]["status"] = "none" line, so the gap was known well enough to be worked around in a tutorial rather than fixed in the adapter. Both are corrected, and the tutorial line is gone because the default now does it. Reported by @Yatsuiii in #331, found by comparing the two adapters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X27MBo6tGVw1UmmchuGFnW
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 #331.
build_trust_record()setappraisal.statusto"affirming"on every record it produced, and__init__had no parameter to change it.appraisal.statusis verifier-owned: section 3.3.1 says a verifier MUST record the depth it actually checked and MUST set the status tocontraindicatedwhen evidence fails, andmodels.Appraisalcarries the same point in a comment, "What this verifier ran, not what the issuer claimed."The record signed and verified, so a consumer reading that field to find out whether anybody checked was told yes by a record nobody had appraised.
Change
appraisal_statusbecomes a constructor parameter defaulting to"none", mirroringTraceSandboxAdapter, which already had this right with tests pinning both the default and the override.agt.py:163was the only remaining hardcoded status in the package.This changes the content of records this adapter emits. An unappraised record now says
nonewhere it used to sayaffirming. That is the correct direction, and a caller who really did appraise declares it withappraisal_status="affirming".Docs that described the old behaviour
docs/integration/agt.mdsaid the adapter "populates anaffirmingappraisal without independently evaluating the session".docs/tutorials/agt-adapter.mdcarried a post-hocrecord["appraisal"]["status"] = "none"line, so the gap was known well enough to be worked around in a tutorial rather than fixed in the adapter. That line is gone because the default now does it, andtests/test_docs_quickstart.pyexecutes that tutorial, so the edit is checked rather than asserted.Verified in both directions
tests/test_agt_adapter.py25 passed, docs quickstart 2 passed, repository 1444 passed.origin/main'sagt.py: 7 of the new cases fail.TrustRecord, and a control that an unappraised record still signs and verifies so the new default costs a caller nothing.test_generators_reproduce_fixtures.pyandtest_safe_integer_range.pyare pre-existing Windows path-separator artifacts.Reported by @Yatsuiii, who found it by comparing the two adapters rather than by reading either one alone.
🤖 Generated with Claude Code
https://claude.ai/code/session_01X27MBo6tGVw1UmmchuGFnW