[lwaldron]

The written shape of protocol tests for review before the validator, checker, and test-protocol skill are built on it (#3).

• ADR 0016, status Proposed: a test is a directory; two evaluation modes; a result.yaml contract with the result shape given to the executor; a run produces a run record, not a review; CI validates tests but does not run them.
• A Tests section in PROTOCOL_STANDARD.md with the schema, the explicit file form ({path} or {url, sha256}), both modes, the result contract, the checker rules, and the run.yaml format.

Where run records are kept is deferred to #44. The eight fixtures merged in waldronlab/agent-protocols#36 are the data this was written against; they will be reformatted to it once the ADR is accepted. Cross-references #5, #10, #22 and #37.

Drafted with Claude Code.

[claude]

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.

Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.

[Copilot]

🟡 Changes recommended

File representation and executor-visible output contracts remain ambiguous, and benchmark-review semantics need reconciliation.

3 open issues

• Define explicit file references instead of inferring paths · New
• Add executor-visible output schema metadata · New
• Separate test availability from review status · New

What changed in this PR

Proposes protocol test contracts before implementing validation and execution tooling, building on existing fixtures and benchmark-review status.

Changes:

• Defines test layout, evaluation modes, and result comparison rules.
• Documents benchmark recording and planned CI responsibilities.
• Adds proposed ADR 0016 to the decision index.

File	Description
PROTOCOL_STANDARD.md	Adds the proposed Tests specification.
docs/​adr/​README.md	Lists ADR 0016 as Proposed.
docs/​adr/​0016-protocol-tests.md	Records test-design decisions, rationale, and consequences.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Schema inconsistencies and incomplete comparison and diagnostic-evidence rules need resolution before implementation.

2 open issues

• Distinguish numeric columns from identifier and text columns · New
• Require independently checkable evidence for diagnostic outcomes · New

2 previously missed issues

In code that hasn't changed since last review

Define serialization and parsing rules for expected-output tables

PROTOCOL_STANDARD.md:372

Please define the supported serialization and parsing rules for expected-output tables. File references do not specify CSV versus TSV, encoding, or delimiter and quoting rules. Two checkers could therefore interpret the same referenced file differently and disagree on pass/fail. Specify supported formats, how the format is selected (including for remote files), and how unsupported formats are rejected.

Clarify mode-specific placement of expected_outputs

docs/​adr/​0016-protocol-tests.md:26

The unconditional five-key contract conflicts with PROTOCOL_STANDARD.md:334–337, which requires diagnostic tests to omit top-level expected_outputs. A validator following the ADR would reject those tests. Describe the four common keys and the mode-specific placement of expected_outputs.

3 resolved since last review

• Add executor-visible output schema metadata
• Define explicit file references instead of inferring paths
• Separate test availability from review status

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Diagnostic field requirements conflict, and table-output and executor-metadata contracts need clarification before implementation.

4 open issues

• Require expected_outputs only for exact-mode fixtures · New
• Distinguish numeric columns from identifier and text columns
• Define an explicit source for executor identity metadata · New
• Require independently checkable evidence for diagnostic outcomes

1 previously missed issue

In code that hasn't changed since last review

Define table serialization and ordered column requirements

PROTOCOL_STANDARD.md:354

The checker requires identical table headers, but the result shape exposes only a file slot, not its ordered column names or serialization format. If the protocol does not prescribe those details, a correct computation can fail against undisclosed headers. The comparison rules also leave delimiter, quoting, and encoding undefined, so CSV and TSV exports have no defined comparison behavior. Define the supported table serialization and expose the required format and ordered columns in each file slot's result shape, while withholding expected data rows.

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Result-shape disclosure and undefined multi-test aggregation need resolution before implementations depend on the contract.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Avoid exposing result-dependent mapping keys

PROTOCOL_STANDARD.md:362

Exposing every nested mapping key can reveal computed answers even when values are withheld. In the variance_missing_values fixture from waldronlab/agent-protocols#36, summary contains A/B/C while non_missing_counts also contains D. Printing those keys reveals that D is excluded from the summary before execution, weakening the benchmark. Distinguish fixed record fields from mappings whose keys depend on the result, and describe those mappings without enumerating the expected keys.

Define suite result layout and run record aggregation

PROTOCOL_STANDARD.md:411

This run record contains multiple tests, but the defined checker command accepts one test directory and reads one result.yaml. No suite result layout or aggregation operation explains how to produce this record, and handling an existing run.yaml on subsequent checks is unspecified. Define either one record per test or a suite layout and aggregation interface, including how subsequent checks affect the record, so implementations produce consistent results.

4 resolved since last review

• Require expected_outputs only for exact-mode fixtures
• Distinguish numeric columns from identifier and text columns
• Define an explicit source for executor identity metadata
• Require independently checkable evidence for diagnostic outcomes

_{Review effort: Balanced}

[claude]

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.

Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.

[Copilot]

🟡 Changes recommended

Evidence requirements, unguided execution, and multi-test recording need consistent contracts before implementation.

1 open issue

• Conditionally provide protocol.md for unguided invocations · New

2 previously missed issues

In code that hasn't changed since last review

Require non-empty assertions in outcome validation

PROTOCOL_STANDARD.md:343

An outcome with expected_outputs: {} or expected_absent: [] satisfies this rule but checks no evidence, so a label and observation can pass by themselves. Require at least one non-empty assertion container to enforce the stated prohibition on label-only outcomes.

Define suite-level result layout and aggregation behavior

PROTOCOL_STANDARD.md:418

The documented checker command accepts one test directory and one result directory containing a single result.yaml, but this record includes two tests. No result layout or aggregation rule explains how the checker obtains both results, or whether successive calls replace or extend run.yaml. Define a suite-level interface and aggregation behavior, or explicitly make records per-test and update the example and ADR accordingly.

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The contract can expose expected answers through result shapes and leaves multi-test run aggregation undefined.

No open issues 🎉

4 previously missed issues

In code that hasn't changed since last review

Avoid revealing omitted mapping keys in withheld outputs

PROTOCOL_STANDARD.md:364

Withholding values does not hide answers encoded in mapping keys. In variance_missing_values from waldronlab/agent-protocols#36, the expected summary contains A, B and C but omits D. Printing those keys reveals which feature is excluded before execution, weakening the benchmark. Distinguish fixed output fields from data-dependent mappings and describe the latter with a value schema rather than their expected membership.

Define multi-test result aggregation and file layout

PROTOCOL_STANDARD.md:418

This record combines two tests, but the documented checker command accepts one test directory and one result directory containing a single result.yaml. No per-test layout or aggregation behavior explains how both results reach run.yaml, including whether successive calls overwrite it. Define either single-test run records or a suite/aggregation interface and result layout so the checker and skill can implement the same contract.

Mark unimplemented CI and checker interfaces as proposed

PROTOCOL_STANDARD.md:280

Please mark this section as proposed and not yet implemented. It describes CI validation and callable checker/skill interfaces as available, but test-schema validation is deferred and neither the checker nor the skill exists yet. A section-level notice would prevent readers from treating passing CI as validation of their fixtures or trying unavailable commands; implementation can remain in follow-up PRs.

Keep fault attribution open for failed comparisons

PROTOCOL_STANDARD.md:442

A failed comparison does not necessarily indicate a protocol or test bug: an agent can execute a correct protocol incorrectly, which is precisely what the model benchmarking in #5 measures. Leave fault attribution open until the executor, environment, protocol, and expectations have been investigated.

1 resolved since last review

• Conditionally provide protocol.md for unguided invocations

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The proposed contract can disclose expected answers and accept incorrect nested output membership.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Withhold data-dependent CSV headers from --shape

PROTOCOL_STANDARD.md:366

Including the expected CSV header in the result shape reveals the answer when output columns are data-dependent. For example, a column-selection test with input columns A, B, C and expected header id,A,C tells the executor which columns to retain before execution. This defeats the same protection provided by withholding nested mapping keys below. Define how fixed, protocol-defined headers are distinguished from data-dependent headers, and withhold the latter from --shape.

Require exact key sets in nested output mappings

PROTOCOL_STANDARD.md:391

Ignoring extra keys in nested mappings lets incorrect output membership pass. For example, a summary containing all expected features plus an unexpected feature would still match. Lines 368–370 explicitly recognize that nested mapping membership can itself be the answer. Limit the extra-key allowance to top-level result fields and require identical key sets in nested data mappings.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Namespace collisions and an inconsistent fixture example need correction before tooling adopts the contract.

1 open issue

• Include all five summaries in the variance_rows fixture example · New

2 previously missed issues

In code that hasn't changed since last review

Reserve diagnostic metadata keys in expected outputs

PROTOCOL_STANDARD.md:386

Diagnostic metadata shares the namespace with expected output keys. An accepted outcome named repaired with expected_outputs: {outcome: success} requires result.yaml.outcome to equal two different values; expected_absent: [outcome] also cannot be satisfied. Reserve outcome and observation in diagnostic expectations and require the validator to reject these collisions.

Reject extension fields colliding with checker-owned keys

PROTOCOL_STANDARD.md:452

Allowing and preserving arbitrary top-level fields in executor.yaml permits tests and checker, which the checker must generate itself. A collision cannot both preserve the supplied value and retain the checker's authoritative value. Reserve the checker-owned run-record keys and require collisions to be rejected before copying extension fields.

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The proposed contract needs clearer evidence requirements, reproducible source provenance, and a deterministic worked example.

No open issues 🎉

3 previously missed issues

In code that hasn't changed since last review

Specify deterministic tie-breaker in example inputs

PROTOCOL_STANDARD.md:307

Include tie_breaker: feature_id_ascending in the example's inputs, as in the variance_rows fixture merged in waldronlab/agent-protocols#36. B and C have identical variances, and the protocol permits any prespecified deterministic tie-breaker. Descending identifiers would correctly remove [A, C] rather than the expected [A, B], so this omission can make a conforming execution fail.

Require at least one non-empty evidence collection

PROTOCOL_STANDARD.md:355

Require at least one non-empty evidence collection. As written, expected_outputs: {} or expected_absent: [] satisfies the field-presence requirement but performs no evidence checks. An accepted outcome label and an arbitrary observation could therefore pass, contrary to this paragraph's intent. Explicitly reject outcomes whose evidence collections are all empty.

Pin and verify the executed source snapshot

PROTOCOL_STANDARD.md:448

Define how the executed source snapshot is pinned and verified. The checker derives protocol_commit from the test checkout, but the contract does not require that checkout to remain unchanged during execution and checking. Results produced with uncommitted edits, or checked after switching revisions, could be attributed to a commit that does not contain the executed protocol/tests. For example, require a clean, pinned checkout, capture its commit before execution, and have the checker verify that snapshot before recording it.

1 resolved since last review

• Include all five summaries in the variance_rows fixture example

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The proposed contract needs safeguards against incorrect execution provenance, conflicting metadata, and exact tests without evidence.

2 open issues

• Capture source revision before execution · New
• Reject executor-supplied uncommitted values · New

1 previously missed issue

In code that hasn't changed since last review

Require non-empty expected_outputs in exact mode

PROTOCOL_STANDARD.md:343

Require a non-empty expected_outputs mapping in exact mode. As written, expected_outputs: {} is allowed and a result.yaml containing {} passes without checking any output. Diagnostic outcomes explicitly reject empty evidence; exact tests need the same safeguard.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The proposed contract needs stronger results-directory and source-attribution guarantees before implementation.

2 open issues

• Require a new or empty results directory for each run · New
• Reject results directories inside the hashed source tree · New

1 previously missed issue

In code that hasn't changed since last review

Record the source snapshot digest in dirty-checkout runs

PROTOCOL_STANDARD.md:460

A dirty-checkout run omits protocol_commit, but run.yaml also contains no source digest. Once the record is attached to an issue without its local source.yaml, different protocol/test contents with the same frontmatter version have indistinguishable source identifiers. Copy the snapshot digest and its algorithm into the run record, and reserve those fields against executor overrides, so the attached record identifies the content checked.

2 resolved since last review

• Reject executor-supplied uncommitted values
• Capture source revision before execution

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No blocking issues remain in this documentation-only proposal, which clearly identifies pending implementation.

No open issues 🎉

2 resolved since last review

• Reject results directories inside the hashed source tree
• Require a new or empty results directory for each run

_{Review effort: Balanced}