Gate the full-set artifact on failure and add the keep_artifacts hatch - #11
Merged
Merged
Conversation
danielhanchen
force-pushed
the
reclaim-fix-aggregate-artifact
branch
from
August 11, 2026 19:08
8d01773 to
f54dd74
Compare
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 #10. A review of that PR found that the reclaim job cannot delete the largest artifact of every run, and that the port dropped an escape hatch it depends on.
1. reclaim could not delete the largest artifact of every run
assembleuploadsunsloth-sd-prebuilt-<tag>(path: dist/*) unconditionally. That aggregate matches no single release asset name, so reclaim always classifies itKEEP. On a green publish its bytes are already release assets, so it is a pure duplicate that survives every cleanup.Measured on run
31289281107: the aggregate is 1,442,716,005 B against the six-bin-artifacts totalling 1,442,714,834 B, identical to within a manifest, plus a 126 MiB source artifact. reclaim was deleting 1.34 GiB of 2.81 GiB, about 48%, and the single biggest item was always the one left behind.llama.cpp gates its equivalent upload
if: ${{ always() && !success() }}, with the reasoning recorded there: uploading it ahead of the publish gate cost storage per run for a bundle nothing reads, and published unverified builds, since any signed-in user can download an artifact from a public repo, for runs that deliberately never released. That gating was missed when this pipeline was ported.This PR applies it, plus
if-no-files-found: warn(an early failure can leavedist/absent, and a missing debug bundle must not turn a diagnosable failure into a confusing second one) andoverwrite: true.!success()rather thanfailure()is deliberate: a cancelled run is exactly when a human wants the rescue bundle. The comment in #10 that presented keeping this artifact as a feature is replaced.2. No way to keep artifacts from a run that publishes nothing
llama.cpp guards its unpublished-deletion step with
&& inputs.keep_artifacts != true; the port dropped it, and this repo has no such input. This repo has no retry workflow either, soassemblebeing skipped after a failed leg meantpublishedwas empty and every successfully built bundle was discarded, with no way to opt out. Apublish:falsedispatch, the documented default for testing, also deleted its own output.This PR adds the
keep_artifactsboolean input and the&& inputs.keep_artifacts != trueguard.Verification
assembleupload step now carriesif: ${{ always() && !success() }}.needs.assemble.outputs.published != 'true' && inputs.keep_artifacts != true.workflow_dispatchinputs are nowref, min_age_hours, publish, keep_artifacts.Note on scope
build.ymlin this repo still has 7 upload steps at the 90-day default retention, including a 563 MiB CUDA runtime zip, and it is where this repo's live artifact storage actually accumulates today. Deliberately out of scope here; worth a separate pass.