diff --git a/PROTOCOL_STANDARD.md b/PROTOCOL_STANDARD.md index d67d230..e9c59a3 100644 --- a/PROTOCOL_STANDARD.md +++ b/PROTOCOL_STANDARD.md @@ -277,6 +277,216 @@ A brand-new protocol, or a release nobody has reviewed yet, still carries the se These rules are enforced by `scripts/validate_protocol.py`, which runs on every pull request. +## Tests + +*Proposed in ADR 0016. The validator, the checker, and the `test-protocol` skill described here +are not yet implemented; until they are, CI does not check `tests/` and the commands below do not +exist. This notice is removed when they do.* + +A protocol may carry tests: known inputs with the outputs a correct execution produces. Tests are +optional. When present they live beside the protocol, one directory per test: + +```text +protocols// +├── protocol.md +└── tests/ + └── / + ├── test.yaml + ├── input.csv # local data referenced from test.yaml + └── retained.csv # an expected output file +``` + +### `test.yaml` + +```yaml +name: variance_rows # must match the directory name +description: Unbiased variance filtering with features in rows. +inputs: # everything the executor is given + data: {path: input.csv} # a file: path, or url + sha256 (below) + filter: variance + theta: 0.4 + tie_breaker: feature_id_ascending +expected_outputs: # what a correct execution produces + summary: + A: 0 + B: 1.3333333333333333 + C: 1.3333333333333333 + D: 33.333333333333336 + E: 48 + removed_features: [A, B] + retained_matrix: {path: retained.csv} # a file, compared as a table +evaluation: + mode: exact # exact | diagnostic + numeric_tolerance: 1.0e-12 # optional, default 0 +``` + +* `name`: (String) Kebab or snake case; must equal the directory name. +* `description`: (String) One sentence. +* `inputs`: (Mapping) Parameter values and data files. Keys are the protocol's own parameter + names where it has them. +* `expected_outputs`: (Mapping) Values and files a correct execution produces. Keys are the + quantities the protocol says to record. +* `evaluation`: (Mapping) `mode` is required. `numeric_tolerance` is an absolute tolerance applied + to every numeric comparison, default `0`. + +A **file** is a mapping with exactly one of two shapes: `{path: }`, inside the test +directory with no `..`, or `{url: , sha256: }`. Any other string is a +literal value, so `data: input.csv` is the string `input.csv`, not a file. The remote form is for +data too large to commit; the digest is verified after download and a mismatch fails the test. Keep +local files small. A table slot in `expected_outputs` may add `header: withheld` when the column +set is itself part of the answer, as in a test that selects columns; the executor then names the +columns as the protocol directs and the checker compares the header as it compares everything +else. The default is to show the header in the result shape. + +### Evaluation modes + +**`exact`**: every entry in `expected_outputs` must be present in the result and match. +`expected_outputs` must have at least one entry; a test that checks nothing is invalid. + +**`diagnostic`**: the input violates a requirement the protocol states, and the test checks what the +executor does about it. `evaluation.accepted_outcomes` lists the outcomes that count as correct. +The top-level `expected_outputs` is omitted; each outcome carries its own evidence instead, at +least one of: + +* `expected_outputs`, compared as in `exact` mode when the executor reports that outcome; +* `expected_absent`, keys that must not appear in the result. This is how a refusal is checked: + an executor that refused did not produce the protocol's outputs. + +An outcome with neither, or with both empty, is invalid, because a label alone is the executor's +word. The keys +`outcome` and `observation` are reserved for the executor's report and may appear in neither +`expected_outputs` nor `expected_absent`; the validator rejects a test that uses them. + +```yaml +evaluation: + mode: diagnostic + accepted_outcomes: + - outcome: orientation_corrected + expected_outputs: + removed_features: [A, B] + retained_features: [C, D, E] + - outcome: refused_for_orientation + expected_absent: [removed_features, retained_features, retained_matrix] +``` + +### The result contract + +An executor, human or agent, is given `inputs`, the **result shape**, and, unless the run is +deliberately unguided (`invocation.kind: unguided` in the run record), `protocol.md`. The result +shape is the top-level keys of `expected_outputs` with every value withheld, each file slot with +its format and, for a table, its header row in order unless the slot says `header: withheld`, the +evaluation mode, and in `diagnostic` +mode the outcome labels with the keys each one expects present or absent. Top-level keys are the +quantities the protocol says to record and are fixed by it. The keys of any nested mapping are +data, such as feature identifiers, and are withheld: the shape shows a nested mapping as +`{: }`, because its membership can itself be the answer. Data rows and scalar values +are never included. `scripts/check_test_result.py --shape ` +prints the shape of every test, or of one with `--test `, and writes `source.yaml` into the +results directory: the commit the checkout is at, whether the protocol directory has uncommitted +changes, and a digest of `protocol.md` and the tests directory. That is the snapshot the run is +attributed to. It is taken before execution because the checkout can move afterwards. + +The executor performs the protocol in any language and writes its results into a **results +directory** of its choosing, one subdirectory per test named after the test, each holding +`result.yaml` and any output files. The results directory must be empty when `--shape` writes +`source.yaml` into it, so a run can never be checked against a previous run's files, and it must +lie outside the protocol directory, whose digest it would otherwise change. The checker refuses +both. The executor does not read `expected_outputs` itself. + +* In `exact` mode, `result.yaml` has one key per key of `expected_outputs`. A file slot is filled + with `{path: }`. +* In `diagnostic` mode, `result.yaml` has `outcome`, one of the accepted outcomes, and + `observation`, one sentence saying what was found. If that outcome carries `expected_outputs`, + their keys follow. + +`scripts/check_test_result.py ` compares every test that has a +result subdirectory, or one with `--test `, and is the only thing that decides pass or fail. A +test with no result subdirectory is reported as not run, which is neither pass nor fail. Its +comparison rules: + +* Numbers match within `numeric_tolerance`. Strings, booleans and nulls match exactly. Lists match + element by element, in order. At the top level of `result.yaml`, a key missing from the result + fails and a key present only in the result is ignored, so an executor may record more than the + test asks. A nested mapping must have exactly the expected key set, because its membership is + data and an extra feature is a wrong answer. +* Files are compared as tables. The only table format is CSV: UTF-8, comma delimited, double-quote + quoting as in RFC 4180, with a header row. A file slot compared as a table must have a name + ending in `.csv`, in `path` or in the URL; the validator rejects anything else, and no other + file format is defined yet. +* Two tables match when the header rows are identical as strings, the rows are in the same order, + and every cell matches. The first column is the row identifier and is compared as a string, as + is the header, so `001` and `1` differ. In every other column a cell is numeric, within + tolerance, when the expected cell parses as a number, and a string otherwise. An empty cell is a + missing value and matches only an empty cell. + +### The run record + +A run is not a review and adds nothing to the protocol. The checker writes `run.yaml` at the top of +the results directory, one record covering every test it compared, overwriting any earlier record +there: + +```yaml +run_record: 1 # schema version of this record +protocol: independent-filtering-variance +version: 1.2.1 +repository: waldronlab/agent-protocols +protocol_commit: 844ce92 # commit the protocol and tests were read at, from source.yaml +uncommitted: false # true, with no commit, if the checkout was dirty +source_digest: sha256:9f2c... # digest of protocol.md and tests/, from source.yaml +started: 2026-10-18T14:02:11Z # UTC timestamps, from the executor +finished: 2026-10-18T14:09:40Z +executor: + agent: claude-code # the agent or person that performed the protocol + harness: Claude Code 2.1.0 # the agent software and version, distinct from the model + model: claude-fable-5-1 # omitted for a human executor + language: R # what the execution was written in, if known + invocation: # how the protocol reached the agent + kind: skill # skill | in-context | unguided + skill: test-protocol 0.1.0 # name and version, for kind: skill + prompt: | # verbatim text that started the run + Run the tests for independent-filtering-variance. +checker: check_test_result.py 0.1.0 +tests: + - name: variance_rows + result: pass # pass | fail + - name: mean_columns + result: pass + outcome: orientation_corrected # diagnostic mode only +``` + +The executor writes `executor.yaml` at the top of the results directory before invoking the checker, +containing the `executor` block and the `started` and `finished` timestamps; the checker copies +them into the record and fills the rest from the test directory and its own version. A missing +`executor.yaml` is an error, not a guess. `protocol_commit` and `uncommitted` come from +`source.yaml`, written by `--shape` before execution. The checker recomputes the digest from the +current checkout and refuses to write a record if it differs, so a record never names a commit +that does not contain what was executed. A run from a dirty checkout records `uncommitted: true` +and no commit. Within `executor`, `agent` is required; `harness`, +`model`, `language`, `invocation`, and `prompt` are optional, since a human executor has no prompt +and a prompt may hold paths or data its author does not want shared. An agent running a skill +fills all of them. `invocation.kind` distinguishes a run through a skill, a run with the protocol +text supplied in context, and a run given no protocol at all; two runs that differ only there are +different experiments. Additional keys, at the top level of `executor.yaml` or +under `executor`, are permitted and preserved, so a study that records cost, token counts, +attempts, or an experimental condition adds them without a schema change. The keys the checker +writes itself, `run_record`, `protocol`, `version`, `repository`, `protocol_commit`, +`uncommitted`, `source_digest`, `checker`, and `tests`, are reserved: an `executor.yaml` that supplies any of them is rejected rather than +silently overridden. `run_record` is incremented when a defined key changes meaning. + +Where run records are kept is not yet defined (issue #44). Attach the record to the pull request or +issue it is relevant to. + +A reviewer who has run the tests may record a `verified-with-benchmark` review (see Review Statuses +above), citing the run record in Notes. That is a judgement by a named person, made once; it is not +made automatically and not made per run. A run with failures is reported where the record is +attached, with the failing comparisons. Which of the executor, its environment, the protocol, or +the test is at fault is found out, not assumed: an agent can execute a correct protocol wrongly, +and measuring how often is what the tests are for. + +CI validates each `test.yaml` against this schema and checks that local files exist. It does not +fetch remote files or execute tests. The `test-protocol` skill in this repository implements the +result contract for an agent. + ## Example Protocol ```yaml diff --git a/docs/adr/0016-protocol-tests.md b/docs/adr/0016-protocol-tests.md new file mode 100644 index 0000000..29917b6 --- /dev/null +++ b/docs/adr/0016-protocol-tests.md @@ -0,0 +1,113 @@ +# 0016. Protocol Tests: Fixtures, a Result Contract, and a Review Status + +- **Status:** Proposed +- **Date:** 2026-10-18 +- **Deciders:** Levi Waldron (User), AI Agent + +## Context and Problem Statement + +Issue #3 proposed a `tests/` directory beside `protocol.md` so that an execution of a protocol can +be checked against known inputs and expected outputs. The first protocol with tests, +`independent-filtering-variance` in `waldronlab/agent-protocols`, now carries eight fixtures merged +ahead of any schema. Their `test.yaml` files agree on `inputs`, `expected_outputs`, and `evaluation` +but improvise the rest, and nothing validates or runs them. + +Three things were undefined: the shape of a test, the shape of the thing a test checks, and what a +run leaves behind. + +## Decision + +### 1. A test is a directory + +`protocols//tests//` holds `test.yaml` and any local data files. `test.yaml` has +`name` (matching the directory), `description`, `inputs`, and `evaluation`; in `exact` mode it also +has `expected_outputs` at the top level, and in `diagnostic` mode the expectations sit under each +accepted outcome instead. A file is always a mapping, `{path: ...}` inside the test directory or `{url: ..., +sha256: ...}` for data too large to commit, so that a bare string is never mistaken for one. The +full schema is in `PROTOCOL_STANDARD.md`. + +### 2. Two evaluation modes + +`exact`: every entry in `expected_outputs` must match the result, numbers within +`numeric_tolerance`. `diagnostic`: the executor must reach one of the listed `accepted_outcomes`, +each of which carries checkable evidence: `expected_outputs`, or `expected_absent` for a refusal, +which the checker verifies by the absence of the protocol's outputs. A label alone is not evidence. +The second mode exists for inputs that violate the protocol's stated requirements, where refusing +and repairing are both correct and silently proceeding is the failure. + +Tables are CSV only, and the first column is an identifier compared as a string. Both are the +simplest rule that fits the existing fixtures; a second format or a per-column type declaration +waits for a test that needs one. + +Custom evaluation scripts, proposed in #3, are not defined. Nothing needs them yet. + +### 3. A result contract + +The executor writes `result.yaml` into a results directory, one subdirectory per test, with +`executor.yaml` and the checker's `run.yaml` at the top. Its keys mirror `expected_outputs`; +file slots are filled with `{path: ...}` in the same directory. In diagnostic mode it also carries +`outcome` and a one-line `observation`. A deterministic checker compares the result to the test. +Without this contract there is nothing machine-checkable, and the agent's own report of success +would be the only evidence. + +The executor is given `inputs` and the result shape: the top-level keys, file slots, mode, and +outcome labels of the test with every expected value withheld, printed by the checker. Keys of +nested mappings are withheld too, since which features appear in a summary can be the answer. It is also +given `protocol.md`, except in a run that is deliberately unguided, which the run record marks as +such so that the baseline condition in #5 is expressible. +It does not read `expected_outputs` itself. Without the shape a correct execution could fail on +a key name; with the values it would not be a test. + +### 4. A run produces a run record, not a review + +A review is a named person's judgement of a protocol. A run is a fact about one execution: which +agent, which model, which day, which tests passed. There will be many runs per version and they +say something different, so a run is not a review and nothing is added to the protocol when one +completes. + +The checker writes a run record, `run.yaml`, naming the protocol and version, each test with its +result, the executing agent, harness and model, the prompt that started the run and how the +protocol reached the agent, the timestamps, and the checker version. The prompt and invocation are +there because the benchmarking study in #5 varies exactly those, and nothing else in the record +tells two such runs apart. The format is in `PROTOCOL_STANDARD.md` because the checker produces it. Where run records are kept is deferred to +#44: while runs are few, the person attaches the record to the pull request or issue they are +working on, and a collection is defined when the benchmarking study in #5 produces volume. + +`verified-with-benchmark` keeps its ADR 0005 meaning. A human who has run the tests, or their own +benchmark, may record it once, as a judgement, citing the run record in Notes. No CI credential +and no automatic review: anyone runs the tests on their own machine with their own agent. + +#22 proposes that trust tier 4 read that review, and #10 asks the runner to surface it. Both +continue to read reviews; neither reads run records. + +### 5. CI validates tests, it does not run them + +`validate_protocol.py` checks each `test.yaml` against the schema and that local files exist. It +does not fetch URLs or execute anything. Running a test needs a model, and a model in CI is a +shared cost and a shared credential; see item 4. + +## Alternatives Considered + +- **Free-form `expected_outputs` compared by an evaluating model.** Rejected: the thing under test + would be grading itself. +- **Hash matching of output files**, as #3 proposed for deterministic steps. Rejected as the primary + mechanism: a trailing newline or a column order fails a correct result. Tables are compared as + tables, with tolerance. +- **A `has_tests` flag in `PROTOCOLS.yaml` and the registry**, as #3 proposed. Deferred to the + registry-schema work in #22. Test availability and a passing run are different facts: tests exist + before anyone runs them. The review records the run; whether the index should also list the tests + is #22's question. +- **Running tests in CI on every pull request.** Rejected for now, per item 5. + +## Consequences + +- Content repositories gain an optional `tests/` directory per protocol; CI rejects a malformed one. +- The eight existing fixtures are reformatted to the schema. Their expected values do not change. +- A `test-protocol` skill is written beside `protocol-runner`, implementing the result contract. + #37 already distinguishes it from the `validate-protocol` skill: that one checks the document + conforms, this one checks an execution gets the right answer. +- The benchmarking study in #5 needs this harness and nothing in it conflicts with #5's design. +- A checker script, `scripts/check_test_result.py`, is the only code that decides pass or fail. +- Running a protocol's tests becomes one way for a reviewer to earn `verified-with-benchmark`, + with the same meaning the status has had since ADR 0005. +- Run records accumulate outside the protocol; their home is #44. diff --git a/docs/adr/README.md b/docs/adr/README.md index 7416608..512b2e9 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -19,5 +19,6 @@ This directory contains Architecture Decision Records (ADRs) for the `agent-prot - [0013. `stable` Is an Assertion by the Authors](0013-stable-is-an-assertion-by-the-authors.md) - [0014. Ask Two Citation Questions, and Require the Answerable One](0014-two-citation-questions.md) — amends 0009 and 0012 - [0015. Migrate Protocol Validator to Python and Pydantic](0015-migrate-validator-to-python.md) +- [0016. Protocol Tests: Fixtures, a Result Contract, and a Review Status](0016-protocol-tests.md) — Proposed