v1.0 stage2: build record schema and helpers - #1264
Merged
Merged
Conversation
The test model directory listed fifteen executables by name and was already missing four, and a build record beside a test model would not have matched any of them. Ignore everything in that directory except the Stan sources instead. R CMD build keeps records out of the tarball by the same rule. Part of #1258.
A build record is a JSON file beside an executable, named from the executable's file name with a leading dot and .cmdstanr.json appended, describing how the executable was built. This adds the format version 1 schema, one validator that the constructor and the reader share, a writer that stages and renames, and a reader that returns why a record cannot be used rather than erroring. The version is checked first and on its own, so a record in a format this cmdstanr does not read is reported as unsupported_format with its version and nothing else, while a record failing any field check is unreadable whole. A record whose artifact hash does not match the executable comes back with nothing it contains. reported_features is encoded by key presence so that enabled, disabled and unknown survive a trip through the file, and is checked for shape but never for membership. Files are hashed with rlang::hash_file(), which raises the rlang floor to 1.0.0. Nothing writes a record beside a user's model yet; that is Stage 3. Part of #1258.
compare_build_records() names every compared field that differs between a record read from disk and one assembled for the current call, in the order the design's table lists them, rather than stopping at the first. Only the fields the table marks compared are consulted: cpp_options after sorting by name, stanc options as an ordered argument vector, the stanc name, the content hashes of the Stan program, its includes in order and make/local, the user header by hash and by path, and the CmdStan installation. Paths are compared as the records hold them; normalising them is the recorder's job. Part of #1258.
The schema helpers collapse to one shape check and one member fetch that takes the shape by name, so the validator reads as the list of fields and their shapes. A missing member and a JSON null both arrive as NULL and are reported as missing. compare_build_records() is now a named list of extractors, one per compared row, applied to both records, so the table in the design note and the code have the same rows in the same order. No check, message or result changes. Part of #1258.
…rsions The comparison now has the artifact row the design's table gives it. The reader's hash check proves a record describes the executable beside it; only comparing the snapshot's hash to the on-disk record's catches another process replacing the executable with an equivalent build, which section 5 says to report as a replaced executable. A JSON object must have unique names. jsonlite writes a repeated name with a numeric suffix, so the writer accepted a record the reader then rejected. The integer check on format_version uses tol = 0, since checkmate's default tolerance let a fractional version through to the exact comparison and reported it as an unsupported format. Found in the Codex review of Stage 2. Part of #1258.
The review deleted the hash check in record_dependency_entry() and the user header and make/local hash comparisons and every test still passed. One test each now fails when those go: a dependency missing its hash is unreadable, a feature written as null is unreadable, a changed header hash at an unchanged path and a changed make/local hash each name their field, and an artifact mismatch reports no format_version. Part of #1258.
The validator now runs the object shape test on the record itself, so a repeated top-level member is unreadable like a repeated member anywhere else. Before, the root only had to be a list. The test for a fractional format_version now uses 1.000000001, which the default integer tolerance accepts and the reader must still reject. Part of #1258.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v1.0 #1264 +/- ##
=======================================
Coverage ? 92.60%
=======================================
Files ? 16
Lines ? 6802
Branches ? 0
=======================================
Hits ? 6299
Misses ? 503
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 participants
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 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.
This PR and the summary below was written with the help of AI coding tools. I have reviewed the code myself.
Stage 2 of the v1.0 compilation-state work (#1258): the build record's format, writer, reader and comparison helper, with the tests that pin their rules. Nothing is wired into the model class and nothing writes a record beside a user's model; that is Stage 3. No user-visible change, so no NEWS entry.
What lands:
R/build_record.R: the format version 1 schema as one validator that the constructor, the writer and the reader all call; a writer that stages the file and renames it into place; a reader that returns why a record cannot be used (missing,unreadable,unsupported_format,artifact_mismatch) rather than erroring; andcompare_build_records(), which names every compared field that differs between two records.format_versionfirst and on its own, so a record in a format this cmdstanr does not read reportsunsupported_formatwith its version and nothing else, while a record failing any field check is unreadable whole. A record whose hash does not match the executable comes back with nothing it contains.reported_featuresis encoded by key presence, so enabled, disabled and unknown survive a trip through the file, and is checked for shape but never for membership.cpp_optionsas canonical Make name to value, stanc options as the emitted argument vector.rlang::hash_file(); the rlang floor rises to 1.0.0.tests/testthat/resources/stan/.gitignoreignores everything but the Stan sources instead of listing executables by name, and.Rbuildignorekeeps records there out of a tarball.Every rule has a sentence in
dev-notes/compilation-state-contract.md(§1, §4, §6) and a test intests/testthat/test-build-record.Rthat fails if the rule is removed.Part of #1238 and #1258. Closes nothing on its own.