diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf33c31..6d7f4ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,6 +11,14 @@ on: permissions: contents: read +# Without this, pushing twice to a branch leaves both runs going and they queue +# behind each other — which matters now that the CUDA jobs hold a runner for +# ~20 minutes. master is excluded from cancellation by keying on run_id there: +# every commit that lands deserves its own verdict, superseded or not. +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref == 'refs/heads/master' && github.run_id || github.ref }} + cancel-in-progress: true + env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true MIX_ENV: test @@ -192,6 +200,59 @@ jobs: - run: mix dialyzer + # Each CUDA leg holds a runner for ~20 minutes, almost all of it nvcc. On a + # pull request that is the price of the gate and it gets paid. On master it + # usually re-confirms a tree the pull request just tested, because a squash + # merge of an up-to-date branch lands exactly that tree. + # + # "Usually" is the whole question: master can move under a branch — v0.8.42 + # landed under this one mid-review — and then the merged tree is genuinely + # untested. So the answer is not "skip master", it is "run master when the + # inputs moved". Anything unexpected resolves to running it. + cuda-scope: + name: CUDA inputs changed? + runs-on: ubuntu-22.04 + outputs: + changed: ${{ steps.q.outputs.changed }} + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + - id: q + env: + EVENT: ${{ github.event_name }} + BEFORE: ${{ github.event.before }} + run: | + set -eu + run_it() { echo "changed=true" >> "$GITHUB_OUTPUT"; echo "$1"; exit 0; } + + # Written as plain `if` blocks rather than `test ... && run_it`: under + # `set -e` a false left-hand side makes the whole && list non-zero, and + # whether that exits the shell is a corner of the standard nobody should + # have to recall while editing CI. + if [ "$EVENT" = "pull_request" ]; then + run_it "pull request: the link job is the gate" + fi + + # A force-push or a first push leaves no usable base. Unknown means run. + case "$BEFORE" in + ''|0000000*) run_it "no usable base commit; running to be safe" ;; + esac + if ! git cat-file -e "${BEFORE}^{commit}" 2>/dev/null; then + run_it "base commit ${BEFORE} not present; running to be safe" + fi + + # vendor/llama.cpp is a submodule, so a bump shows up as a change to + # that path — which is exactly the case that must not be skipped. + if git diff --name-only "$BEFORE" "$GITHUB_SHA" | tee /tmp/changed.txt \ + | grep -qE '^(Makefile|c_src/|mix\.exs|mix\.lock|vendor/llama\.cpp|\.github/workflows/ci\.yml)'; then + run_it "a CUDA build input changed" + fi + + echo "changed=false" >> "$GITHUB_OUTPUT" + echo "no CUDA build input in this push; skipping the link job" + cat /tmp/changed.txt + # Nothing in CI ever selected the CUDA backend, which is how a NIF that could # not resolve `cuMemCreate` reached a release. Runners have no GPU, so this # cannot execute a kernel; it proves the two things that were actually broken: @@ -200,7 +261,8 @@ jobs: cuda-link: name: CUDA link (cu${{ matrix.major }}) runs-on: ubuntu-22.04 - needs: [setup] + needs: [setup, cuda-scope] + if: needs.cuda-scope.outputs.changed == 'true' strategy: fail-fast: false matrix: diff --git a/CHANGELOG.md b/CHANGELOG.md index b68210f..b8032b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,15 @@ the GPU, and passes the smoke suite: **528 tests, 0 failures**. Only reachable on a partial accept, which is why a working MTP setup can still post plausible acceptance rates while quietly drifting. + + Cross-checked against upstream's own reference path rather than reasoned about + alone. `common_sampler_sample_and_accept_n` (`common/sampling.cpp`) returns the + tokens sampled at batch indices `0..k`, and `tools/server/server-context.cpp` + then does `slot.prompt.tokens.insert({ids.begin(), ids.end() - 1})` followed by + `slot.sampled = ids.back()` — appending every accepted token *except the last*, + and carrying the last into the next iteration. That is the invariant this fix + restores. (Upstream trims with `seq_rm` where this binding rolls back and + re-decodes, so the mechanisms differ; the resulting context must not.) - **The CUDA NIF could not be loaded** — `ggml-cuda.a` leaves the CUDA runtime, cuBLAS/cuBLASLt and the CUDA driver API unresolved, but the Linux link line only ever added `-lstdc++ -lm -lpthread`. The resulting `.so` linked and then @@ -94,6 +103,17 @@ the GPU, and passes the smoke suite: **528 tests, 0 failures**. with `ldd -r` — pointing the loader at the toolkit's driver stub, which carries the `libcuda.so.1` soname, so a GPU-less runner can still perform a real resolution. `enif_*` is excluded, being supplied by the BEAM at load. + + Each leg holds a runner for about twenty minutes, almost all of it nvcc, so + the job runs on every pull request but on master only when a CUDA build input + moved — `Makefile`, `c_src/`, `mix.exs`, `mix.lock`, the `vendor/llama.cpp` + submodule, or the workflow itself. A squash merge of an up-to-date branch + lands the exact tree the pull request tested; the case worth re-testing is + master having moved underneath it, which is what that path list detects. A + force-push, a missing base commit, or anything else unexpected resolves to + running the job. The workflow also gained a `concurrency` group so repeated + pushes to a branch cancel their predecessors instead of queueing behind them, + keyed on `run_id` for master so every landed commit keeps its own verdict. - **Precompiler unit tests** — `test/precompiler_test.exs` pins the artifact selection rules, including the case that motivates the driver check.