Skip to content

unsloth: prove the merged tree still has the features, and that it compiles #175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielhanchen merged 2 commits into from
Sep 4, 2026
Merged

unsloth: prove the merged tree still has the features, and that it compiles #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7828fbc
unsloth: prove the merged tree still has the features, and that it co…
Sep 3, 2026
c21aa43
unsloth: build mtmd in the compile gate too, not just llama
danielhanchen Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/unsloth-pin-preflight.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,33 @@ jobs:
if ! python3 ../scripts/unsloth/merge_checks.py --root . ; then
PROBLEMS="${PROBLEMS}- the merged tree builds, but \`scripts/unsloth/merge_checks.py\` found a resolution that is silently wrong. See the run log for file and line.\n"
fi

# The other half of that question. merge_checks.py asks whether the
# tree contains something wrong; this asks whether it still contains
# what each pin carries. A pin that has rotted into a no-op, or an
# arch registration a resolution quietly dropped, is invisible to
# every other check here and to the compiler.
if ! python3 ../scripts/unsloth/pin_contract.py --root . --base "$BASE" \
--pr-set ../scripts/unsloth/pr-set.json --report "${RUNNER_TEMP}/pin_contract.json" ; then
Comment on lines +176 to +177

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check only pins the preflight actually merged

When the set contains a closed optional pin and at least one other pin merges, the loop above skips the optional pin, but this invocation reloads the entire pr-set.json. pin_contract.py consequently checks code that was deliberately not merged and reports it missing, turning an otherwise successful preflight into a failure and triggering the repin automation. Pass an effective pin list here, as the nightly does with --prs-json.

Useful? React with 👍 / 👎.

PROBLEMS="${PROBLEMS}- the merged tree is missing code a pin carries. See the run log for the pin and file.\n"
fi
NOTES="$(jq -r '.notices[]?' "${RUNNER_TEMP}/pin_contract.json" 2>/dev/null || true)"
if [ -n "$NOTES" ]; then
PROBLEMS="${PROBLEMS}- pins upstream has taken over, safe to delete from \`pr-set.json\`:\n\n\`\`\`\n${NOTES}\n\`\`\`\n"
fi

# A clean merge is not a compiling tree. On 09-03 ggml-org#27754
# merged with no conflicts at all and did not compile: upstream had
# added a parameter to build_attn_mha and the pin's new
# build_attn_sparse still called the old signature. Nothing above
# can see that. CPU only and the `llama` target only, which is where
# that translation unit lives; 59s cold at -j4 with no ccache.
if ! cmake -B "${RUNNER_TEMP}/gate" -DCMAKE_BUILD_TYPE=Release \
-DGGML_CUDA=OFF -DLLAMA_BUILD_TESTS=OFF -DLLAMA_BUILD_SERVER=OFF \
-DLLAMA_BUILD_EXAMPLES=OFF -DLLAMA_BUILD_TOOLS=OFF -DLLAMA_CURL=OFF > /dev/null \
|| ! cmake --build "${RUNNER_TEMP}/gate" --target llama -j "$(nproc)" ; then
Comment on lines +193 to +194

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Build mtmd in the preflight compile gate

When a pin changes tools/mtmd and only that target fails to compile, this preflight remains green because it disables tools and builds only llama, while the compile gate in .github/workflows/unsloth-prebuilt.yml explicitly builds both llama and mtmd. This defeats the earlier preflight for the exact class of mtmd failure described in the nightly workflow, so the problem is discovered only when the nightly starts. CMake's local help confirms the semantics: --target <tgt>... builds the named targets instead of the defaults.

Useful? React with 👍 / 👎.

PROBLEMS="${PROBLEMS}- the pins merge cleanly and the merged tree does not compile. See the run log for the file and line; this is the failure that only shows up in the CUDA leg once the nightly has fanned out.\n"
fi
fi

if [ -z "$PROBLEMS" ]; then
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/unsloth-pr-set-lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ jobs:
scripts/unsloth/test_pin_merge.py \
scripts/unsloth/test_merge_checks.py \
scripts/unsloth/test_sync_deletes.py \
scripts/unsloth/test_carry_vintage.py; do
scripts/unsloth/test_carry_vintage.py \
scripts/unsloth/test_pin_contract.py; do
echo "::group::$t"
python3 "$t" || fail=1
echo "::endgroup::"
Expand Down
50 changes: 43 additions & 7 deletions .github/workflows/unsloth-prebuilt.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: AGPL-3.0-only
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.

Expand Down Expand Up @@ -282,12 +282,10 @@
# .github/workflows, which upstream history routinely does).
if [ "$EXISTS" != "true" ] || [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
git remote add upstream https://github.com/ggml-org/llama.cpp.git
# Checking out the upstream base replaces this working tree with
# upstream's, which has no scripts/unsloth/. Keep the resolver
# somewhere the checkout cannot take away.
ADDITIVE_MERGE="${RUNNER_TEMP}/additive_merge.py"
cp scripts/unsloth/additive_merge.py "$ADDITIVE_MERGE"
cp scripts/unsloth/merge_checks.py "${RUNNER_TEMP}/merge_checks.py"
# The upstream checkout below takes scripts/unsloth/ away. Copy the
# whole dir out, not file by file: see the note above the step.
cp -r scripts/unsloth "${RUNNER_TEMP}/us"
ADDITIVE_MERGE="${RUNNER_TEMP}/us/additive_merge.py"
if [ "$(jq length <<<"$PRS")" != 0 ]; then
# Merges need a merge-base, so unshallow first.
if [ "$(git rev-parse --is-shallow-repository)" = "true" ]; then
Expand Down Expand Up @@ -448,15 +446,53 @@

# A bad pin resolution can still build fine, so it must be caught before the source artifact ships. See merge_checks.py.
# Its own step, not more script in `resolve`: GitHub caps one workflow string at 21000 chars and that step is near it. See check_workflow_scalars.py.
# That is also why `resolve` copies all of scripts/unsloth/ to ${RUNNER_TEMP}/us in one line rather than one cp per script: every check added
# here would otherwise cost another line inside the capped block, and going over silently disables the whole workflow.
- name: Check the merged tree for silently wrong resolutions
if: ${{ env.MERGED_PINS == '1' }}
run: |
set -euo pipefail
if ! python3 "${RUNNER_TEMP}/merge_checks.py" --root . ; then
if ! python3 "${RUNNER_TEMP}/us/merge_checks.py" --root . ; then
echo "::error::the pinned PRs merged, but merge_checks.py found a resolution that is silently wrong; see the log for file and line" >&2
exit 1
fi

# merge_checks.py asserts the ABSENCE of two known-bad shapes. This asserts the PRESENCE of what each pin carries, which is a different question and
# the one that goes unanswered when a pin rots into a no-op or a resolution quietly drops an arch registration. Free, so it runs before the compile gate.
- name: Check every pin still contributes what it carries
if: ${{ env.MERGED_PINS == '1' }}
# Through env, never interpolated into the script: `prs` carries PR
# titles, which are third-party text, and `${{ }}` pastes them into the
# shell source before bash ever sees it.
env:
PRS: ${{ steps.r.outputs.prs }}
BASE: ${{ steps.r.outputs.base }}
run: |
set -euo pipefail
if ! python3 "${RUNNER_TEMP}/us/pin_contract.py" --root . --base "$BASE" \
--prs-json "$PRS" --report "${RUNNER_TEMP}/pin_contract.json" ; then
echo "::error::the pinned PRs merged, but the merged tree is missing code a pin carries; see the log for the pin and file" >&2
exit 1
fi

# The gap this closes, observed 09-03: ggml-org#27754 merged with zero conflicts and did not compile, because upstream had added a parameter to
# build_attn_mha and the pin's new build_attn_sparse still called the old signature. Nothing before this point can see that, and without it the release
# dies in the CUDA leg after the 38-job fan-out. CPU only: a cold `llama` build took 59s at -j4 with no ccache, against 20-60 minutes for a CUDA build.
# mtmd is in the gate because `llama` alone is not enough: observed 09-04, ggml-org#25731 built `llama` clean while tools/mtmd did not compile at all,
# upstream having made mtmd_image_preprocessor::preprocess const while the pin's Inkling subclass stayed non-const, so it overrode nothing and the
# vision and audio towers were abstract. Every vision pin lands in mtmd, so a gate that skips it cannot see the whole class.
- name: Compile gate (CPU, llama and mtmd targets)
if: ${{ env.MERGED_PINS == '1' }}
run: |
set -euo pipefail
cmake -B "${RUNNER_TEMP}/gate" -DCMAKE_BUILD_TYPE=Release \
-DGGML_CUDA=OFF -DLLAMA_BUILD_TESTS=OFF -DLLAMA_BUILD_SERVER=OFF \
-DLLAMA_BUILD_EXAMPLES=OFF -DLLAMA_BUILD_TOOLS=ON -DLLAMA_CURL=OFF > /dev/null
if ! cmake --build "${RUNNER_TEMP}/gate" --target llama mtmd -j "$(nproc)" ; then
echo "::error::the pinned PRs merged cleanly and the merged tree does not compile; fix or drop the pin rather than letting the build matrix find this" >&2
exit 1
fi

# The stamped source tree (every build): every build child extracts this
# instead of cloning, and assemble ships it as the release's source-tarball
# asset, so a source build reproduces the same fingerprinted binary. Only
Expand Down
Loading
Loading