Skip to content

fix(ci) make manual speed profiles trustworthy #678

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
davide221 merged 1 commit into from
Sep 7, 2026
Merged

fix(ci) make manual speed profiles trustworthy #678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
67 changes: 50 additions & 17 deletions .github/workflows/speed-profile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
name: Speed Profile

# Report-only speed profile for the inference engine. Runs on the self-hosted
# RTX 3090 (lucebox-rtx3090) on PRs that touch the engine or the optimizations, and on
# manual dispatch. It NEVER blocks a PR (continue-on-error: true) — it publishes a
# report to the run summary + uploads the JSON / markdown / nsys trace as artifacts.
# Speed profile for the inference engine. Runs on the self-hosted RTX 3090
# (lucebox-rtx3090) on PRs that touch the engine or the optimizations, and on manual
# dispatch. PR runs are report-only; manual runs fail when the runner is not ready or
# the performance result is missing, noisy, or regressed. Every successful run publishes
# a report to the run summary + uploads the JSON / markdown / nsys trace as artifacts.
#
# Why report-only: perf has run-to-run variance (thermals, clocks, scheduling).
# Gating a merge on a noisy absolute number produces false failures. We surface the
Expand All @@ -30,7 +31,9 @@ jobs:
name: Speed profile (self-hosted RTX 3090, sm_86)
runs-on: [self-hosted, lucebox-rtx3090]
timeout-minutes: 30
continue-on-error: true # report-only: a slow/failed profile must not block the PR
# Keep automatic PR profiling advisory, but make a manual promotion check honest:
# setup/profile failures and invalid performance evidence must return a red run.
continue-on-error: ${{ github.event_name == 'pull_request' }}

# Model paths live on the runner, not in the repo (multi-GB weights). They are
# overridable via repo variables so the runner owner can point at whatever is
Expand Down Expand Up @@ -64,8 +67,8 @@ jobs:
run: |
# The weights are staged on the self-hosted runner out of band. If they are
# absent the engine binary aborts with a cryptic gguf "No such file" error, so
# check up front and SKIP cleanly instead — this job is report-only, and a
# missing model on the runner is an environment issue, not a PR defect.
# check up front. A PR run skips cleanly because a missing runner asset is not
# a PR defect; a manual promotion check fails because it produced no evidence.
target="$MODELS/$TARGET_MODEL"
draft="$MODELS/$DRAFT_MODEL"
present=true
Expand Down Expand Up @@ -94,6 +97,10 @@ jobs:
echo "Available draft candidates under \`$MODELS\`:"
find "$MODELS" -maxdepth 4 -type f \( -name '*.gguf' -o -name '*.safetensors' \) -print 2>/dev/null | sort | sed 's/^/ - /' || true
} >> "$GITHUB_STEP_SUMMARY"
if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then
echo "::error title=Speed profile unavailable::Model weights not found under $MODELS — see the run summary."
exit 1
fi
echo "::warning title=Speed profile skipped::Model weights not found under $MODELS — see the run summary."
fi

Expand Down Expand Up @@ -131,17 +138,21 @@ jobs:
if: steps.models.outputs.present == 'true'
run: |
cd server
# Use a committed baseline if one is staged so the report can flag a
# regression. Seed it once from a green `main` run's profile.json artifact
# (see docs/specs/speed-profile.md); without it the delta is simply skipped.
# The path is overridable so the runner owner can point elsewhere.
# Use the runner-staged baseline so a PR cannot weaken its own comparison.
# Seed it from a stable `main` run's profile.json artifact and configure
# LUCEBOX_SPEED_BASELINE (see docs/specs/speed-profile.md). The repository
# path remains a convenient fallback for local/report-only use.
baseline="${LUCEBOX_SPEED_BASELINE:-scripts/speed-baseline.json}"
baseline_arg=()
if [ -f "$baseline" ]; then
baseline_arg=(--baseline "$baseline" --regress-pct "${LUCEBOX_SPEED_REGRESS_PCT:-0.10}")
echo "Regression check against baseline: $baseline"
else
echo "No baseline at $baseline — regression flagging disabled this run."
if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then
echo "::error title=Speed baseline missing::No baseline at $baseline; a manual promotion check cannot detect regressions."
exit 1
fi
echo "::warning title=Speed baseline missing::No baseline at $baseline — regression flagging disabled this PR run."
fi
# Use 128 generated tokens per prompt by default: long enough to reduce
# startup/noise effects while keeping the serialized 3090 queue bounded.
Expand Down Expand Up @@ -174,16 +185,27 @@ jobs:
echo "Profiler produced no report (the run failed earlier — see logs)." >> "$GITHUB_STEP_SUMMARY"
fi

- name: Flag losslessness / regressions (annotations, non-blocking)
- name: Validate profile result
if: always() && steps.models.outputs.present == 'true'
run: |
[ -f server/profile.json ] || exit 0
# Report-only: emit warnings, never fail. A losslessness FAIL means the fast
if [ ! -f server/profile.json ]; then
if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then
echo "::error title=Speed profile missing::The profiler produced no profile.json artifact."
exit 1
fi
echo "::warning title=Speed profile missing::The profiler produced no profile.json artifact."
exit 0
fi
# A losslessness FAIL means the fast
# path changed the output and it is NOT run-to-run noise (AR agreed with
# itself) — worth triaging (real bug vs batched-verify FP). Inconclusive
# prompts (engine intrinsically nondeterministic) are NOT failures.
# prompts (engine intrinsically nondeterministic) are NOT failures. Keep this
# advisory until the profiler records the first-divergence logit gap and can
# distinguish a near-tie FP flip from a logic bug.
python3 - <<'PY'
import json
import os

d = json.load(open("server/profile.json"))
ll, reg, noise = d.get("lossless", {}), d.get("regression", {}), d.get("summary", {}).get("noise", {})
if ll and not ll.get("lossless", True):
Expand All @@ -197,6 +219,17 @@ jobs:
print(f"::warning title=Noisy speed profile::{','.join(noise.get('metrics', []))} exceeded "
f"the relative stddev threshold ({noise.get('threshold_rsd', 0)*100:.1f}%). "
"Treat small deltas as below the profiler detection threshold.")
if os.environ.get("GITHUB_EVENT_NAME") == "workflow_dispatch":
failures = []
if not reg:

@cubic-dev-ai cubic-dev-ai Bot Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: When profile.json contains a truthy regression object but lacks summary.noise or headline metrics, this manual gate accepts it as valid evidence. Validate the required report schema and metric values before applying the regression and noise checks.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/speed-profile.yml, line 224:

<comment>When `profile.json` contains a truthy `regression` object but lacks `summary.noise` or headline metrics, this manual gate accepts it as valid evidence. Validate the required report schema and metric values before applying the regression and noise checks.</comment>

<file context>
@@ -197,6 +219,17 @@ jobs:
                     "Treat small deltas as below the profiler detection threshold.")
+          if os.environ.get("GITHUB_EVENT_NAME") == "workflow_dispatch":
+              failures = []
+              if not reg:
+                  failures.append("the baseline comparison is missing")
+              elif reg.get("regressed"):
</file context>

failures.append("the baseline comparison is missing")
elif reg.get("regressed"):
failures.append("a performance regression exceeded the configured threshold")
if noise.get("noisy"):
failures.append("the measurements were too noisy for a promotion decision")
if failures:
print(f"::error title=Speed profile rejected::{'; '.join(failures)}.")
raise SystemExit(1)
PY

- name: Reset GPU clocks
Expand All @@ -212,5 +245,5 @@ jobs:
server/profile.json
server/profile.md
server/profile.nsys-rep
if-no-files-found: warn
if-no-files-found: ${{ github.event_name == 'workflow_dispatch' && 'error' || 'warn' }}
retention-days: 30
36 changes: 26 additions & 10 deletions docs/specs/speed-profile.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,20 @@ and they stay fixed so every run is comparable over time:
variance (thermals, clock boosting, scheduler jitter), then reports mean and
stddev for the headline metrics. Use `--reps 3` for a faster smoke profile when
needed, but PR comparisons should prefer 5+.
- **`--noise-rsd-pct 0.05`** — report-only noise threshold. If any tracked
- **`--noise-rsd-pct 0.05`** — noise threshold. If any tracked
headline metric has relative stddev above 5%, the markdown calls the profile
**NOISY** and tells reviewers to treat small deltas as below the profiler
detection threshold.
detection threshold. Automatic PR profiles report this as a warning; a manual
promotion profile fails because noisy measurements are not valid release evidence.

**Rule:** keep these consistent. A delta vs the baseline is only a valid regression
signal if both runs used the same config — if you ever change a parameter, re-seed the
baseline (you cannot compare across configs). When baseline and current 1σ intervals
overlap and the delta is smaller than `--regress-pct`, the report marks that row as
**noisy / overlap** instead of inviting reviewers to chase a ghost regression. All of
these states are warnings only; the profiler remains report-only.
**noisy / overlap** instead of inviting reviewers to chase a ghost regression. These
states are warnings on automatic PR profiles. A manual dispatch is the promotion check:
it fails on a missing runner model, missing baseline/report, noisy result, or a
performance regression outside the configured threshold.

## Losslessness gate (and why a bit-exact compare is too strict on its own)

Expand Down Expand Up @@ -88,7 +91,9 @@ path genuinely changed the output — but that is still not proven a logic bug:
the batched-verify FP effect above (verify scores draft tokens as a batch vs AR
one-at-a-time). Classifying a FAIL as bug-vs-FP needs the **logit gap** at the first
mismatch (near-tie = FP, clear gap = bug) — a follow-up the binaries don't emit yet. CI
surfaces a FAIL as a non-blocking `::warning::` for triage; it stays report-only.
surfaces a FAIL as a non-blocking `::warning::` for triage, including on manual
promotion profiles. It cannot safely become a blocking signal until the binaries expose
the logit gap at the first mismatch and the profiler can classify FP near-ties.

## CI settings

Expand All @@ -98,6 +103,13 @@ temporarily override those values with repo variables `LUCEBOX_SPEED_N_GEN`,
`LUCEBOX_SPEED_REPS`, and `LUCEBOX_SPEED_NOISE_RSD_PCT`, but PR-to-PR comparisons
should keep them fixed.

Automatic pull-request runs are advisory and never block a merge. A manual dispatch is
strict enough to use as promotion evidence: it requires the staged target/draft models,
an existing baseline at `LUCEBOX_SPEED_BASELINE`, a complete JSON report, stable
measurements, and no regression beyond `LUCEBOX_SPEED_REGRESS_PCT` (10% by default).
Correctness divergences remain explicit warnings for the reason described above; use the
normal GPU correctness suite as the blocking correctness gate.

## Run it locally

```bash
Expand All @@ -114,8 +126,9 @@ python3 scripts/profile.py \

## Comparing against a baseline

The profiler is **report-only**, but it can diff the current run against a saved
profile so reviewers see a single regression table instead of two separate reports.
The profiler can diff the current run against a saved profile so reviewers see a single
regression table instead of two separate reports. The delta is advisory on PR events and
blocking on manual promotion runs.
The comparison is a JSON round-trip:

1. **Capture a baseline.** Run the profiler on the reference commit and keep its
Expand All @@ -125,9 +138,12 @@ The comparison is a JSON round-trip:
python3 scripts/profile.py ... --out-json scripts/speed-baseline.json
```

Commit `scripts/speed-baseline.json` so every later run compares against the same
reference. Re-seed it whenever you change a profiler parameter (`--budget`,
`--n-gen`, `--reps`, …): you cannot compare across configs.
For CI, stage that file outside the checkout on the self-hosted runner and set the
repository variable `LUCEBOX_SPEED_BASELINE` to its absolute path. Keeping the
promotion baseline outside the PR checkout prevents a candidate from weakening its
own comparison. `scripts/speed-baseline.json` remains the local fallback. Re-seed the
runner baseline from a reviewed `main` run whenever you intentionally change a
profiler parameter (`--budget`, `--n-gen`, `--reps`, …): configs cannot be compared.

2. **Compare a later run.** Point `--baseline` at that file and set the regression
threshold:
Expand Down
Loading