Skip to content

Fix the three Codex findings on #2 - #4

Merged
devYRPauli merged 5 commits into
mainfrom
fix/pr2-codex-findings
Oct 14, 2026
Merged

devYRPauli merged 5 commits into
mainfrom
fix/pr2-codex-findings

Conversation

@devYRPauli

Copy link
Copy Markdown
Owner

Problem

The Codex review on #2 reported three defects. All three are still on main.

  • P1, runner.rs: a run with --scenarios records the corpus as wic-50 revision v1. Two different scenario sets therefore share one label.
  • P2, result.rs: validate accepts documents that the published schemas reject. On main, validate exits 0 for each of these ten documents:
    • v3 with failure_class: null, cause: null, http_status: 0, forced: false, ports: [] or parameter_count_b: -1
    • v3 with no evidence_path
    • v2 with failure_class: "bogus"
    • v1 with a run_id or a cause
  • P2, export.rs: evidence_path and cause.reference accept absolute local paths. The site copies them into results.json, results.csv and the HTML links.

Fix

  • The runner names a corpus by its content. A run of the frozen catalog keeps wic-50 and v1. Any other scenario set gets id custom, and its revision is the corpus SHA-256. The measurement contract spec states this rule under T1.2.
  • Every result is checked against the schema of its declared version. The check runs after serde, so the existing serde messages do not change. This moves jsonschema from dev-dependencies to dependencies. Cargo.lock does not change.
  • annotate refuses a v1 result. The v1 schema has no cause field, so an annotated v1 file fails its own schema.
  • evidence_path must be a relative path under evidence/. cause.reference must be an http:// or https:// URL or a relative repository path, with an optional #fragment. Each path segment must be non-empty, must not start with ., and must use only ASCII letters, digits, ., _ and -. The fragment uses the same characters.
  • The site link builders no longer trim a leading /, because the validator now refuses one. One site fixture had a leading / in a reference. I removed it. The golden output does not change.
  • The runner records failure.http_status only from 100 to 599, as RFC 9110 defines. An endpoint can answer 600 to 999. The v3 schema refuses those values, so without this change run writes a file that the new check refuses. The failure code stays http_error, and the evidence keeps the raw status.
  • Both result writers check the serialized document against its schema before they write it. A writer and the schema can no longer drift apart without an error.
  • README and CONTRIBUTING now say which versions each command accepts.

Testing

  • cargo fmt --check and cargo clippy --workspace --all-targets -- -D warnings are clean.
  • cargo test --workspace: 150 tests pass. main has 141.
  • Each of the ten documents above exits 0 on main and exits 2 on this branch.
  • All 187 tracked result files pass willitcall validate.
  • willitcall site --results results/ gives output that is byte-identical to main.
  • The CI results job (Python Draft202012Validator), the tools unittest and the redaction ledger check pass. The ledger check verifies 6 runs and 300 transcripts.
  • A run against a mock endpoint that answers 600 writes a result that validate accepts.
  • I removed each new check in turn and ran the tests. A test fails for each removal.
  • The release binary grows from 7.9 MB to 12.3 MB because jsonschema is now a runtime dependency. site over results/ takes 0.05 s. main takes 0.04 s.

devYRPauli and others added 5 commits October 12, 2026 15:16
run_measurement copied the id and revision of the frozen wic-50 v1
catalog into every run, but it hashed the scenarios that actually ran.
A run with --scenarios therefore claimed to be wic-50 v1. Default runs
made the same claim, because the embedded set no longer matches the v1
hash since 964f6e5.

A run now keeps the catalog id and revision only when its corpus hash
equals the catalog hash. Any other corpus records id "custom" and its
hash as the revision. The M7 spec states the rule under T1.2.

The evidence test pinned the old label for a one-scenario corpus. It
now expects "custom". A new test runs the frozen catalog scenarios and
expects wic-50 v1. The CLI test checks the full corpus block of a
--scenarios run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
validate deserialized a result with serde and checked only the totals.
Serde accepts values that the published schema rejects, so validate
printed "valid" for a failure_class of null, a failure.http_status of 0,
or a preflight_override with forced false. The same gap existed for v1
and v2: a v2 failure_class of "bogus" and a v1 run_id passed too.

The parser now checks the raw document against the embedded schema for
its version, after serde so the serde messages stay the same. The check
covers validate, site, annotate and rescore, because all of them parse
through it. The hand-written v2 property check is removed, because the
v2 schema covers it. jsonschema moves from a dev-dependency to a
dependency. Cargo.lock does not change.

annotate now refuses a v1 result. The v1 schema has no cause field, so
the edited file always failed its own schema.

All 187 tracked result files still validate, and the generated site is
byte-identical. The release binary grows from 7.9 MB to 12.3 MB.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The parser accepted any string in evidence_path. The site then wrote it
verbatim into results.json and results.csv, and linked it from the
index and the appendix. A result with /Users/alice/... therefore
published the author's home directory. annotate --reference had the
same defect: it accepted /Users/alice/notes.md, and validate and site
both passed the edited file.

validate_result and validate_measurement now check both fields. Every
parse runs one of them, and every write runs them first. An
evidence_path must be a relative path under evidence/. A cause
reference must be an http(s) URL or a relative repository path. Each
path segment must be non-empty, must not start with '.', and may use
only ASCII letters, digits, '.', '_' and '-'. The check does not use
std::path, because a Unix host does not see C:\ as absolute.

All 187 tracked result files pass. One site fixture had a leading '/'
on its evidence path. It is fixed, and the site golden does not change.
The leading '/' trims in evidence_url and reference_url are removed,
because no such value reaches them now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An endpoint can answer with a status from 600 to 999. The runner
recorded that status in failure.http_status, but the v3 schema allows
100 to 599 only. Since the readers check the schema, run wrote a file
that validate, site, annotate and rescore then refused.

RFC 9110 defines status codes 100 to 599 only. The runner now records
http_status only in that range. The failure code stays http_error, and
the evidence keeps the raw status.

Both result writers now check the serialized document the way the
readers do, before they write it. A writer and the schema can no longer
drift apart without a loud error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reference check applied the path segment rule to the whole value.
It refused a case-study link with a section anchor, such as
docs/case-studies/x.md#section, and a line anchor such as
runner.rs#L770. The base accepted both and rendered working links.

The check now splits off one '#' fragment. The path part keeps the
segment rule. The fragment allows ASCII letters, digits, '.', '_' and
'-' only, so it cannot carry a local path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devYRPauli
devYRPauli merged commit 427e2a4 into main Oct 14, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant