fix(evidence): complete the fixture diagnostic contract - #811
Conversation
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
`fix(evidence): retain failed fixture trace count` made the JSON report fall back to the traced case count when a run fails, but left this test asserting the field is absent. Assert the count instead. A consumer needs `evaluatedCases` to be an integer unconditionally. Without it a failing fixture is indistinguishable from a broken toolchain: both produce a document with no count. Nine is the fixture's own number, not the failure output's. `no-match` is the ninth case in `products/evidence/fixtures/acceptance/adult-status/fixtures/cases.yaml`, so the mutated run reaches nine cases before it stops. The count is also asserted against `cases.len()`, which is the invariant a reader depends on. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
`print_diagnostics` rendered the relayed trace with `unwrap_or_default`, so a rendering failure would have printed `(no output captured)` through `indented`. That is a different fact from an absent trace, and an operator reading the diagnostic could not tell the two apart. The trace is parsed out of `evidence` standard output before it is stored, so re-rendering it cannot fail. State that instead of defaulting past it. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
`--case` on `evidence evaluate`, `--fixture`/`--case` on `evidencectl fixtures run`, and the reworded `--explain` help changed the public Clap command tree. Every reference page carries the catalog SHA-256, so the whole catalog moves when any command does, and the committed pages still held the previous digest and the previous `--explain` text. Regenerated with `node scripts/generate-cli-reference.mjs` from `docs/site`. No page is hand-edited. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cf01e9992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if selected_case.is_some_and(|selected| selected != id) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
When --case selects a case that produces no signed evidence, such as no-match in the adult-status acceptance fixture, the other successful cases are skipped here but the fixture-wide privacy_expectation.evidence_contains check still runs against the now-empty successful_values. Consequently, a case whose own expected outcome passes exits with fixture required disclosure is absent; single-case runs need to avoid full-suite presence assertions or evaluate them against case-appropriate data.
Useful? React with 👍 / 👎.
| evaluated_cases: summary | ||
| .ok() | ||
| .map(|summary| summary.evaluated_cases) | ||
| .or_else(|| (trace.case_count() > 0).then(|| trace.case_count())), |
There was a problem hiding this comment.
When evaluation fails after all real cases have settled—for example, during the final privacy-expectation check—trace.fail appends the synthetic (fixture) entry, and this fallback counts it as an evaluated case. A selected no-match run therefore reports evaluatedCases: 2 even though only one fixture case ran; derive the failure count from real case attempts rather than the total trace-entry count.
Useful? React with 👍 / 👎.
| let trace = structured_trace(&stdout); | ||
| StepOutcome { | ||
| passed: true, | ||
| evaluated_cases: evaluated_cases(&stdout), | ||
| stdout, | ||
| evaluated_cases: structured_evaluated_cases(trace.as_ref()) | ||
| .or_else(|| evaluated_cases(&stdout)), |
There was a problem hiding this comment.
When an --explain evaluation exits successfully but emits malformed or incompatible JSON—for example, because --evidence-bin points to a mismatched wrapper or binary—structured_trace returns None, yet this branch still marks the fixture as passed and silently omits the requested trace. This makes a broken diagnostic toolchain indistinguishable from an unexplained successful run; explained evaluations should fail unless the required structured document and case count parse successfully.
Useful? React with 👍 / 👎.
| diagnose_reference_match( | ||
| trace, | ||
| expected, | ||
| context.bundle, |
There was a problem hiding this comment.
For a reference case with entityReferenceCount, this diagnosis runs before the actual count comparison, reducing both expected and observed values to the same List class. If the derived list contains the wrong number of entity references, the later count check fails without updating the diagnosis, so the failed case has no findingCodes entry despite an authored expectation disagreeing; record or override the finding when that count comparison fails.
Useful? React with 👍 / 👎.
| None if expected.contains_key("outputGate") || expected.contains_key("error") => { | ||
| ResultClass::ServiceUnavailable | ||
| } |
There was a problem hiding this comment.
When a reference case states an exact error without also repeating publicProblem, this branch classifies every expectation as service-unavailable, even though validate_reference_error maps errors such as source_protocol_error to source.unavailable and derivation-input failures to evidence.unavailable. As a result, passing cases such as truncated-pager in the DHIS2 reference fixture emit a spurious public-problem-mismatch; derive the expected class from the exact error and execution context instead of assigning the service class wholesale.
Useful? React with 👍 / 👎.
1 participant
What this changes
Evidence fixture diagnostics currently stop at a verdict. A failing fixture
names the contract that broke and nothing else, so an adopter learns that a
case failed but never how far it got or what the run actually observed. This
completes the diagnostic contract in three parts.
A closed expected-versus-observed diagnosis per case. Each case in the
--explain --explain-format jsondocument now carries a result drawn from aclosed set (
match,no-match,ambiguous,evidence-unavailable,source-unavailable,service-unavailable,bundle-refused,selector-refused), a bounded classification of any matched value(
boolean-true,integer,structured, and so on), areasonCodenamingthe evaluator outcome that produced the observation, and
findingCodesnaming the authored expectation that disagreed.
A case count that is always present.
evaluatedCaseswas emitted only ona passing run. It is now the number of cases the run got through in both
outcomes, so a failing document is distinguishable from a document produced by
a broken toolchain. Both were previously a document with no count.
Selecting one fixture or one case.
evidence evaluate --case <id>, andevidencectl fixtures run --fixture <path> [--case <id>]. A selector thatnames nothing referenced by the project is refused.
evidencectl fixtures run --explainnow asksevidencefor the structuredform and relays it, instead of asking for the text form and relaying the
string. It totals
evaluatedCasesfrom those documents and interprets noEvidence semantics of its own, which keeps the adopter-tooling boundary in
AGENTS.mdintact.Breaking change
evidencectl fixtures run --jsonemits each fixture'straceas a JSONobject rather than as a string holding the text rendering.
evidencectlisadopter tooling outside the frozen Version 1 runtime contract, and the field
only ever appears under
--explain.Security-sensitive review notes
This touches data minimization, so the trace surface is the review
subject.
closed enumeration. None carries a source, a selector, SQL, a credential, or
a governed result value.
categoryClassesidentifies an allowed result by its zero-based conceptposition in the requirement and its zero-based value position in that
concept's captured governed codelist, and only after the output is proven to
belong to that codelist. An arbitrary string stays the shape
string. Thetrace never substitutes raw category text or a concept identifier for an
ordinal.
diagnosticsExcludecanaries are still checked against bothrendered forms on every run, including a run that stopped on an error.
a_failing_run_is_refused_when_its_trace_holds_a_declared_canaryandan_explained_fixture_run_discloses_no_protected_selector_valuecover this.select_fixture_pathsrefuses anunknown selector without rendering the rejected value back.
fixture_selection_refuses_non_exact_names_without_rendering_themcoversthis.
print_diagnosticsno longer renders the relayed trace withunwrap_or_default. A default there printed(no output captured), whichis a different fact from an absent trace, and an operator could not tell the
two apart.
Owning areas
The change spans
crates/,products/, anddocs/site/, which theone-owning-area rule normally discourages. Both non-crate areas are
consequences of the crate change rather than separate work:
docs/site/src/content/docs/reference/cli/**anddocs/site/src/data/generated/cli-reference.jsonare generated outputs.AGENTS.mdrequires regenerating them with the change, not as a follow-up.Every reference page carries the catalog SHA-256, so adding any flag moves
the whole catalog; all 80 pages plus
cli-reference.jsonare regenerated bynode scripts/generate-cli-reference.mjs, none hand-edited.products/evidence/reference/request-adapter/deployment-projects/FIXTURES.mdis held in parity with the CLI surface by
products/evidence/scripts/check-config-key-paths.sh.Verification
Run on the pinned
1.95.0toolchain that CI uses, not the host default:cargo fmt --checkcargo check --locked --workspace --all-targetscargo clippy --workspace --all-targets -- -D warningscargo test --locked --workspaceproducts/evidence/scripts/check-contracts.shproducts/evidence/scripts/check-source-neutrality.shproducts/evidence/scripts/check-verifier-portability.shproducts/evidence/scripts/check-config-key-paths.shproducts/evidence/scripts/check-authoring-schema.shproducts/evidence/scripts/check-authoring-no-io.shdocs/sitenpm run check:cli-referenceThree
registry-minthealthcheck tests(
server::tests::healthcheck_*_over_a_live_socket) fail intermittently on aloaded machine. They are unrelated to this change, which touches no
registry-mintsource: the helper atcrates/registry-mint/src/server.rswraps a full live-socket round trip in a hard
Duration::from_secs(1)andpanics with
Elapsed(())when the budget is missed. Three consecutive runs ofcargo test --locked -p registry-mint --libon this branch gave 148 passed,148 passed, then 146 passed with 2 failed. Flagged rather than fixed, since it
is outside this change's scope.