Repository navigation
Make sync-artifacts unable to half-publish a benchmark run - #8
Merged
Merged
Conversation
Three times in the 2026-08 campaigns the uploader printed "uploaded N, skipped 0" while shipping an artifact that could not be reproduced. A crash is loud and stops you; this produced a green log, a real HF URL, and a submission with correct numbers. Everything you would check as a sanity test passed. All of it came from one line: collect_artifacts's add() dropped any path that did not exist, so "not present" and "not applicable" were the same thing. Pass the resolved recipe_path to collect_artifacts. cmd_sync_artifacts computed a fallback for the manifest and then handed collect_artifacts args.recipe instead, so a sync without --recipe wrote a manifest naming a recipe and uploaded no recipe. That is the whole reason three published runs cite a recipe_sha256 whose file exists nowhere. Collect checkpoints from the flat <root>/<dataset>/<weight file> layout as well as .runs. That flat layout is what --weights-root documents and what the evaluator itself reads; only .runs was collected, so a weights root built any other way uploaded zero checkpoints in silence. The uploader and the evaluator disagreed about where checkpoints live. Refuse to publish an incomplete checkpoints/all tier. Every dataset with a stats.json must ship a checkpoint, and the recipe and versions lock must be present. The results tier only reports, so ordinary syncs stay non-brittle. Verify the recipe against the hash the submission claims. The fallback resolves the PACKAGED family recipe, which is the wrong file for any run that used a custom --recipe; uploading it would label the run with a recipe that did not produce it. Shipping the wrong recipe is worse than shipping none: it turns a visible gap into an invisible contradiction. Claude-Session: https://claude.ai/code/session_01D8uB5n3e1zYjix18gT2Y4w
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 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.
Follow-up to #7. That PR guarded wrong numbers; this one guards incomplete artifacts.
Three times in the 2026-08 campaigns the uploader printed
uploaded N, skipped 0while shipping something unreproducible. A crash is loud and stops you. This produced a green log, a real HF URL, and a submission with correct numbers — everything you'd check as a sanity test passed. The only way I caught it was enumerating the repo afterwards.All of it traced to one line in
collect_artifacts:"not present"and"not applicable"were the same thing.What
recipe_path.cmd_sync_artifactscomputed a fallback for the manifest, then handedcollect_artifactsargs.recipeinstead. A sync without--recipewrote a manifest naming a recipe and uploaded none. This is the actual root cause of three published runs citing arecipe_sha256whose file exists nowhere.<root>/<dataset>/<weight file>layout too. That is what--weights-rootdocuments and what the evaluator reads. Only.runswas collected, so any other weights root uploaded zero checkpoints silently. The uploader and evaluator disagreed about where checkpoints live.checkpoints/alltier. Every dataset with astats.jsonmust ship a checkpoint; recipe and versions lock must be present.resultsonly reports, so ordinary syncs stay non-brittle.--recipe.Why 4 matters most
Fixing 1 alone would make things worse. The fallback would start uploading
recipes/rf100vl/yolox.jsonas provenance for runs that actually used a custom recipe — labelling a run with a recipe that did not produce it. Shipping the wrong recipe is worse than shipping none: it converts a visible gap into an invisible contradiction. 1 and 4 have to land together.There's a matching asymmetry this closes: the harness already refuses to call a submission valid when the recipe hash is missing (that is how it caught a corrected run this morning), but happily published when the recipe file was missing. It validated the pointer and never checked the referent.
Check
tests/test_publish_completeness.py(new, 7 tests) written against the three real failures: flat-layout checkpoints are collected,.runsstill works, a trained dataset without a checkpoint raises, a missing recipe raises,resultstier only reports, a mismatched recipe raises, a matching one is accepted.Full harness suite: 153 passed.
One existing test (
test_collect_tiers_add_weights_progressively) needed a recipe fixture — it exercises tier layering withtier="checkpoints", which the contract now requires a recipe for. Behaviour under test is unchanged.Not verified
stats.jsonas "this dataset trained". A campaign that writes checkpoints without stats would not be checked.IncompletePublishis a hard failure for programmatic callers ofcollect_artifactsat publishable tiers, not just the CLI. That is deliberate, but it is an API behaviour change.collect_now()closure that passes neithereval_dirnorsubmissions_dir, which is why auto-sync runs carry no predictions or submissions. That looks intentional (the standalone sync is the publishing step) but it is worth confirming.