Repository navigation
Assert format: date-time on runs and 1.0 runs[]: a non-RFC 3339 started_at / ended_at is a schema error - #52
Merged
Merged
Conversation
8000
…ed_at / ended_at is a schema error The served schemas mark a run's `started_at` / `ended_at` (and the same fields in `runs[]` of an Eval 1.0 body) `"format": "date-time"`, but the validators were built for draft 2020-12 without opting in to format assertion, so `format` was only an annotation. A run with `"started_at": "yesterday"` was accepted, kept the string in its body and left the `runs.started_at` column NULL, so it sorted and filtered as a run with no timestamp, while the query side already refused the same literal with `type_mismatch` (#47). The core validators are now built with `should_validate_formats(true)`. A bad value is one `schema` error at its path (`/started_at`, `/ended_at`, `/runs/{i}/started_at`), on `PUT .../runs/{id}`, in `runs:batch` and on `POST /evals/...` with a 1.0 body. The crate's `date-time` check is RFC 3339 with a `T` and an offset, so a value that passes also parses with `DateTime::parse_from_rfc3339`: an accepted run with a timestamp always has the column. The separator must be `T` (RFC 3339 §5.6): a space, which chrono parses and 0.3.0 therefore accepted, is now a `schema` error. The other formats in the generated schemas are the numeric hints schemars emits (`double`, `int64`, `uint32`, `uint64`). jsonschema 0.56 does not know them and ignores unknown formats by default; that is now set explicitly, so they stay annotations. Card and Eval 2.0 have no date-time field. The store test that pinned the old behaviour is replaced by one that expects the refusal and one that every accepted timestamp form fills the column. The validate and store runs module docs say how formats and timestamps are handled. Stored bodies are not re-validated; the rule applies to writes. Closes #47 Signed-off-by: Yutaka Nishimura <ytk.nishimura@gmail.com>
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
3167
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.
format: date-timeis enforced: a non-RFC 3339started_at/ended_atis aschemaerrorFixes #47. The served JSON Schema has declared
format: date-timeon run timestamps since 0.1.0, and nothing checked it: the validators were built for Draft 2020-12, whereformatis an annotation unless assertions are switched on. A run with"started_at": "yesterday"was accepted, the string stayed in the body, and theruns.started_atcolumn was NULL, so the run sorted and filtered as one with no timestamp.Behaviour
compile()inevalhub-corebuilds every validator withshould_validate_formats(true)(andshould_ignore_unknown_formats(true), the crate default, made explicit). Adate-timethat is not RFC 3339 is422with oneschemaerror at the field:/started_at,/ended_at, or/runs/N/started_atfor anevalhub.eval/1.0body.PUT …/runs/{id},POST …/runs:batchandPOST /evals/…(1.0) refuse it and write nothing.DateTime::parse_from_rfc3339parses (offsets, fractional seconds, lower-caset/z, the UTC leap second).T(ort), as RFC 3339 §5.6 has it. A space is refused, although chrono would parse it, so a body 0.3.0 accepted with a space is now aschemaerror.date-timebecomes an assertion. The other formats in the served schemas (double,int64,uint32,uint64, schemars' numeric hints) are unknown to jsonschema 0.56 and stay annotations.card.jsonandeval-2.jsonhave nodate-timefield, so records are affected only through the 1.0runs[].Verification
cargo fmt --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test -p evalhub-core(64),-p evalhub-store(97, Docker),-p evalhub-server(84, Docker); no insta snapshot changed. Core RED was observed before the fix (invalid values produced no error); the store test that pinned the old behaviour flipped to failing after the fix and was rewritten.just semver-check/just packagenot run (no version bump in this PR).