Prebuilt: dry-run the pin merges before the nightly - #53
Conversation
whisper.cpp slim bundles ship no libggml* and load against the ggml of a
paired llama install, so they need an exact ABI identity for the ggml they
were compiled against. Today they use the -mix- suffix of our tag, on the
assumption that it is a ggml commit. It is not: resolve computes it as
SETHASH="$(jq -r 'map("\(.repo)#\(.number):\(.sha)")|join("\n")' | sha256sum | cut -c1-7)"
which hashes the pinned PR set only. The base tag is the prefix, not part of
the hash, so while pins are stable the suffix is constant as the base and the
real ggml move underneath it. In the published release list 53618c5 covers 8
releases spanning b9909 to b10001, 2d6bd50 and 1f1aaa4 cover 6 each. Those
really are different ggml trees:
b10173 8f3c6e1
b10181 e96ffb0
b10225 740c55b
so a consumer pairing on the suffix concludes "still compatible" across spans
where ggml changed by up to ~92 upstream builds.
Records `git rev-parse HEAD:ggml` in the manifest instead. A tree id changes
only when ggml/ contents change, so a release touching nothing under ggml/
correctly does not force a downstream rebuild. ggml_version is recorded
alongside it but is not an ABI key on its own: it reads 0.18.0 for all three
trees above.
Nothing consumes these yet; whisper.cpp switches over separately.
unsloth-pr-set-lint.yml checks that pins are well formed and belong to their PR, which catches a bad edit. It cannot catch the failure that actually recurs: a pin that was fine yesterday and stops merging today because the base tag moved under it. That is what killed 08-02, and between the two causes the nightly failed four times in a week, every time discovered by hand well after the fact. Runs the same merge the nightly will run, on the same base tag it will pick, a few hours ahead of the schedule. On a conflict it files an issue naming the pin, the conflicting files and the hunks, so the repin can happen before the schedule burns 39 build jobs on a resolve that was always going to refuse. Stops at the first conflicting pin rather than probing the rest. Later pins merge onto a tree missing this one, so their conflicts are consequences of it and reporting them is noise. Detection only. Mirroring reviewed pins to refs/pins so a force-push cannot prune them needs a token with workflow scope, since upstream history routinely touches .github/workflows and GITHUB_TOKEN is refused; that waits for the App token. Verified against both real cases: the old pin set on b10229 reports the LLM_ARCH_INKLING conflict in src/llama-arch.cpp, and the current pin set on b10235 passes, which is what tonight will build.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e9278761b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fi | ||
| continue | ||
| fi | ||
| if ! git fetch -q --no-tags "https://github.com/${SRC}.git" "$SHA" 2>/dev/null; then |
There was a problem hiding this comment.
When a PR is force-pushed but the old commit remains fetchable from another ref, this fetch and merge succeed even though the nightly rejects the SHA because it is no longer listed by the PR. The actual resolver performs that membership check in .github/workflows/unsloth-prebuilt.yml lines 199-207, so the preflight can report success and close its tracking issue shortly before the nightly fails; mirror the resolver's /pulls/${NUM}/commits check here.
Useful? React with 👍 / 👎.
| - name: Resolve base and dry-run the merges | ||
| id: p | ||
| run: | | ||
| set -uo pipefail |
There was a problem hiding this comment.
If cloning, fetching the base tag, or checking it out fails transiently, the shell continues because errexit is disabled. A failed checkout can therefore probe against the clone's default branch, while a failed clone/CD can eventually leave no pins to process, after which lines 89-93 publish status=success and may close a real conflict issue. Fail the step or explicitly record a failure unless the requested base was successfully checked out.
Useful? React with 👍 / 👎.
| AGE_H="${UNSLOTH_LLAMA_MIN_RELEASE_AGE_HOURS:-6}" | ||
| CUTOFF="$(date -u -d "-${AGE_H} hours" +%s)" | ||
| BASE="$(gh api 'repos/ggml-org/llama.cpp/releases?per_page=100' \ | ||
| --jq "[.[] | select(.draft==false and .prerelease==false) | select((.published_at|fromdateiso8601) <= ${CUTOFF})] | max_by(.published_at|fromdateiso8601) | .tag_name")" |
There was a problem hiding this comment.
Because this workflow runs at 16:47 UTC while .github/workflows/unsloth-prebuilt.yml runs at 20:13 UTC, independently applying a six-hour age cutoff does not select the same base. Any release published between 10:47 and 14:13 UTC is too young here but eligible by the nightly, so the preflight tests the previous tag and can report success even when the newly eligible tag conflicts with a pin. Compute the cutoff for the anticipated nightly run or schedule the probe so both resolutions use the same eligibility boundary.
Useful? React with 👍 / 👎.
| if [ "$REQUIRED" != "false" ]; then | ||
| PROBLEMS="${PROBLEMS}- \`${SRC}#${NUM}\` is **${STATE}** and is required; the nightly will refuse.\n" | ||
| fi | ||
| continue |
There was a problem hiding this comment.
When a required pin is closed or merged, the nightly resolver exits immediately, but this continue omits that pin from the probe tree and tries every later pin anyway. If a later change depends on the omitted pin, the alert reports a secondary merge conflict that the nightly never reaches and may direct maintainers to repin the wrong entry; break after recording a required-pin failure while retaining continue only for explicitly optional pins.
Useful? React with 👍 / 👎.
1 participant
unsloth-pr-set-lint.ymlchecks that pins are well formed and belong to their PR, which catches a bad edit. It cannot catch the failure that actually recurs: a pin that was fine yesterday and stops merging today because the base tag moved under it.That is what killed 08-02. Between that and pins being force-pushed away, the nightly failed four times in a week, and every one was found by hand well after the fact.
What it does
Runs the same merge the nightly will run, onto the same base tag it will pick, a few hours before the schedule. On a conflict it files an issue naming the pin, the conflicting files and the hunks, so the repin can happen before the schedule burns 39 build jobs on a resolve that was always going to refuse.
It stops at the first conflicting pin rather than probing the rest. I had it report everything at first, and on the real 08-02 data that produced this:
Only the first is real. The other two merge onto a tree missing Inkling, so they are consequences of it.
resolveexits on the first failure and this now matches.Verification
Both real cases, against the actual tags:
Old pin set on b10229, the 08-02 breakage:
Current pin set on b10235, which is what tonight will build:
Also
bash -non the run block, and the URL parsing checked against all four live entries.Not included
Mirroring reviewed pins to
refs/pins/<sha>so a force-push cannot prune them, which is the other half of the 07-31 failure. That needs a token withworkflowscope: upstream history routinely touches.github/workflowsandGITHUB_TOKENis refused such a push. It waits for the App token that the whisper chaining also needs.