Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 11 additions & 8 deletions .github/workflows/nightly-bfcl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
# checkpoint/sampling. Track B of docs/proposals/2026-06-10-bfcl-nightly-parser-
# verification.md. Informational/non-blocking — never a merge gate.
#
# Runs nightly (full non-live set) and, on PRs that touch this pipeline, a quick
# non-live sanity subset for merge confidence. The A/B mechanics were validated
# manually on an H100 box (see scripts/bfcl/README.md "Validation status"); the
# PR trigger doubles as the live-CI-runner shakeout.
# Runs nightly over non_live + live + multi_turn (static, reproducible; excludes
# the agentic/executable categories that need web-search/memory/sandbox infra).
# On PRs that touch this pipeline it runs a quick non-live sanity subset for
# merge confidence. The A/B mechanics were validated manually on an H100 box
# (see scripts/bfcl/README.md "Validation status").
name: Nightly BFCL

on:
Expand All @@ -32,7 +33,7 @@ on:
categories:
description: "Comma-separated BFCL test categories"
required: false
default: "simple_python,simple_java,simple_javascript,multiple,parallel,parallel_multiple,irrelevance"
default: "simple_python,simple_java,simple_javascript,multiple,parallel,parallel_multiple,irrelevance,live_simple,live_multiple,live_parallel,live_parallel_multiple,live_irrelevance,live_relevance,multi_turn_base,multi_turn_miss_func,multi_turn_miss_param,multi_turn_long_context"
vllm_tool_parser:
description: "pure-vLLM --tool-call-parser"
required: false
Expand Down Expand Up @@ -94,16 +95,18 @@ jobs:
needs: build-wheel
if: ${{ !cancelled() && needs.build-wheel.result == 'success' }}
runs-on: 4-gpu-h100 # TP=2 per arm, both arms concurrent (GPUs 0,1 + 2,3)
timeout-minutes: 120
# Generous: the nightly set adds multi_turn (multi-turn state simulation),
# which is much slower than the single-turn AST/live categories.
timeout-minutes: 240
permissions:
contents: read
env:
ROUTER_LOCAL_MODEL_PATH: /models
HF_HOME: /models
MODEL: ${{ github.event.inputs.model || 'Qwen/Qwen3.6-27B' }}
BFCL_MODEL_FC: ${{ github.event.inputs.bfcl_model || 'Qwen/Qwen3.6-27B-FC' }}
# PR = quick non-live sanity subset; schedule/dispatch = full non-live set.
CATEGORIES: ${{ github.event.inputs.categories || (github.event_name == 'pull_request' && 'simple_python,irrelevance' || 'simple_python,simple_java,simple_javascript,multiple,parallel,parallel_multiple,irrelevance') }}
# PR = quick non-live sanity subset; schedule/dispatch = non_live + live + multi_turn.
CATEGORIES: ${{ github.event.inputs.categories || (github.event_name == 'pull_request' && 'simple_python,irrelevance' || 'simple_python,simple_java,simple_javascript,multiple,parallel,parallel_multiple,irrelevance,live_simple,live_multiple,live_parallel,live_parallel_multiple,live_irrelevance,live_relevance,multi_turn_base,multi_turn_miss_func,multi_turn_miss_param,multi_turn_long_context') }}

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 Avoid ambiguous BFCL score matches when adding live categories

When the default nightly set includes both parallel/multiple and live_parallel/live_multiple, the existing score parser in scripts/bfcl/run_ab.py searches with score_root.rglob(f"*{category}_score.json"). That pattern is ambiguous: the parallel row can also match BFCL_v4_live_parallel_score.json, and multiple can match parallel_multiple/live_multiple, so the nightly report and regression gate can attribute the wrong accuracy depending on traversal order. Please make the score lookup exact (or group-aware) before running mixed non-live and live categories in one job.

Useful? React with 👍 / 👎.

steps:
Comment on lines +108 to 110

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail the workflow when any requested category is missing from the report.

At Line 109, the nightly scope expands to 17 categories, but scripts/bfcl/run_ab.py currently computes overall from only categories with discovered score files. If one of the new categories produces no score artifact, the job can still pass with partial coverage.

Suggested guard step (after `run_ab.py`)
       - name: Launch arms + run official BFCL A/B
         env:
@@
           python scripts/bfcl/run_ab.py \
             --baseline  "vllm=$A_URL" \
             --candidate "smg=$B_URL" \
             --bfcl-model "$BFCL_MODEL_FC" \
             --categories "$CATEGORIES" \
             --bfcl "$(command -v bfcl)" \
             --project-root "$RUNNER_TEMP/bfcl_ab" \
             --out "$RUNNER_TEMP/bfcl_ab.md" \
             --json-out "$RUNNER_TEMP/bfcl_ab.json" \
             --tolerance 0.02 || echo "BFCL_AB_REGRESSION=1" >> "$GITHUB_ENV"
+      - name: Verify all requested categories produced scores
+        if: success()
+        run: |
+          python - <<'PY'
+          import json, os, sys
+          req = [c.strip() for c in os.environ["CATEGORIES"].split(",") if c.strip()]
+          p = os.path.join(os.environ["RUNNER_TEMP"], "bfcl_ab.json")
+          with open(p, encoding="utf-8") as f:
+              payload = json.load(f)
+          rows = {r["category"]: r for r in payload.get("per_category", [])}
+          missing = [
+              c for c in req
+              if c not in rows or rows[c].get("baseline") is None or rows[c].get("candidate") is None
+          ]
+          if missing:
+              print("Missing BFCL scores for categories: " + ", ".join(missing), file=sys.stderr)
+              sys.exit(1)
+          PY
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/nightly-bfcl.yml around lines 108 - 110, The nightly
workflow expands to 17 categories in the CATEGORIES environment variable, but
the run_ab.py script only computes results from categories with discovered score
files, allowing the job to pass even if one category produces no score artifact.
Add a validation step after the run_ab.py execution that checks whether all
requested categories from the CATEGORIES environment variable were actually
processed and included in the report, and fail the workflow if any are missing.
This ensures complete coverage before allowing the job to succeed.

- uses: actions/checkout@v6
- uses: actions/download-artifact@v8
Expand Down
98 changes: 19 additions & 79 deletions scripts/bfcl/README.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
# BFCL nightly A/B — `scripts/bfcl/`

**Track B** of the parser-verification proposal
(`docs/proposals/2026-06-10-bfcl-nightly-parser-verification.md`): run the
**official** Berkeley Function Calling Leaderboard (`bfcl-eval`) against two
serving "arms" and diff the scores. The companion **Track A** (offline,
deterministic parser-conformance gate) lives in `crates/parser_conformance/`.
**Track B** of the parser-verification proposal (`docs/proposals/2026-06-10-bfcl-nightly-parser-verification.md`): run the **official** Berkeley Function Calling Leaderboard (`bfcl-eval`) against two serving "arms" and diff the scores. The companion **Track A** (offline, deterministic parser-conformance gate) lives in `crates/parser_conformance/`.

## The experiment

Two arms expose an identical OpenAI `/v1` endpoint. The same official `bfcl`
CLI (FC mode) is pointed at each; **everything is held fixed except the
frontend**, so any score delta is attributable to the tokenization + parsing
layer — the number that argues for an engine adopting SMG's frontend.
Two arms expose an identical OpenAI `/v1` endpoint. The same official `bfcl` CLI (FC mode) is pointed at each; **everything is held fixed except the frontend**, so any score delta is attributable to the tokenization + parsing layer — the number that argues for an engine adopting SMG's frontend.

| | baseline | candidate |
|---|---|---|
Expand All @@ -20,12 +13,7 @@ layer — the number that argues for an engine adopting SMG's frontend.
| who parses tool calls / reasoning | vLLM (`--tool-call-parser hermes`) | SMG (`--tool-call-parser qwen`) |
| model · engine · checkpoint · sampling | **identical** | **identical** |

**Why FC mode is mandatory.** BFCL's `…-FC` model handlers send the native
`tools` param and score `response.choices[].message.tool_calls` — i.e. the
*server's parsed output*. The non-FC (prompt) handlers format tools into the
prompt and parse the text themselves, bypassing the server parser. Only FC mode
puts SMG's / vLLM's parser on the critical path, so the driver always uses the
`-FC` handler (e.g. `Qwen/Qwen3-4B-Instruct-2507-FC`).
**Why FC mode is mandatory.** BFCL's `…-FC` model handlers send the native `tools` param and score `response.choices[].message.tool_calls` — i.e. the *server's parsed output*. The non-FC (prompt) handlers format tools into the prompt and parse the text themselves, bypassing the server parser. Only FC mode puts SMG's / vLLM's parser on the critical path, so the driver always uses the `-FC` handler (e.g. `Qwen/Qwen3-4B-Instruct-2507-FC`).

## Files

Expand Down Expand Up @@ -66,13 +54,9 @@ B_URL=$(BFCL_GPU=2,3 BFCL_SMG_TOOL_PARSER=qwen_xml BFCL_SMG_REASONING_PARSER=qw
bash launch_arm.sh stop
```

Key env knobs for `launch_arm.sh`: `BFCL_GPU` (CUDA_VISIBLE_DEVICES, e.g. `0,1`),
`BFCL_TP` (tensor-parallel size — match the GPU count), `BFCL_MAX_MODEL_LEN`,
`BFCL_{VLLM,SMG}_{TOOL,REASONING}_PARSER`, and `BFCL_VLLM_EXTRA` for extra vLLM
flags.
Key env knobs for `launch_arm.sh`: `BFCL_GPU` (CUDA_VISIBLE_DEVICES, e.g. `0,1`), `BFCL_TP` (tensor-parallel size — match the GPU count), `BFCL_MAX_MODEL_LEN`, `BFCL_{VLLM,SMG}_{TOOL,REASONING}_PARSER`, and `BFCL_VLLM_EXTRA` for extra vLLM flags.

`run_ab.py` exits non-zero if the candidate's overall accuracy drops more than
`--tolerance` (default 2pp) below the baseline.
`run_ab.py` exits non-zero if the candidate's overall accuracy drops more than `--tolerance` (default 2pp) below the baseline.

## Per-model parser flags (vLLM ~0.22.x)

Expand All @@ -86,51 +70,23 @@ flags.
| Kimi K2 | `kimi_k2` / — | `kimik2` / `kimi` |
| MiniMax M2 | `minimax_m2` / `minimax_m2` | `minimax_m2` / `minimax` |

> The mid-2026 SKUs (DeepSeek V4, Kimi K2.6, Qwen3.6, MiniMax M2.7) may use newer
> parser names; confirm against the installed vLLM build:
> `vllm serve --help | grep -A40 tool-call-parser`.
> The mid-2026 SKUs (DeepSeek V4, Kimi K2.6, Qwen3.6, MiniMax M2.7) may use newer parser names; confirm against the installed vLLM build: `vllm serve --help | grep -A40 tool-call-parser`.

## Gotchas discovered while bringing this up (read before debugging)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider alternative phrasing for "bringing this up."

LanguageTool flagged "Gotchas discovered while bringing this up" as potentially overused phrasing. Consider a more specific alternative such as "while setting this up" or "while getting this running."

🧰 Tools
🪛 LanguageTool

[style] ~75-~75: ‘Bringing this up’ can be overused. To elevate your writing, try opting for an alternative.
Context: ...l-parser. ## Gotchas discovered while bringing this up (read before debugging) - **bfcl-eval...

(BRING_THIS_UP)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/bfcl/README.md` at line 75, In the README.md file, replace the phrase
"bringing this up" in the section header with a more specific and clear
alternative such as "setting this up" or "getting this running." Locate the
section header that reads "## Gotchas discovered while bringing this up (read
before debugging)" and update it to use the more descriptive phrase to improve
clarity and avoid overused terminology.

Source: Linters/SAST tools


- **`bfcl-eval` needs `soundfile`.** Its Qwen handler imports `qwen_agent` →
`soundfile`; without it `bfcl --help` itself crashes. `pip install soundfile`.
- **Cap the context.** Qwen3-4B defaults to a 256K `max_model_len` → ~36 GiB KV
cache → engine init OOM. Pass `--max-model-len 16384` (the launch helper
defaults to this); use the **same** value on both arms.
- **Install `ninja` in the vLLM env (do NOT reach for `--enforce-eager`).**
vLLM's torch.compile / CUDA-graph path shells out to `ninja` to build kernels
(required for newer archs like Qwen3.6's `qwen3_5`); if it's missing the
engine dies with `No such file or directory: 'ninja'`. `--enforce-eager` only
*hides* this by skipping compilation (slower). Real fix:
`pip install ninja` in the vLLM env **and put its bin on `PATH`** (vLLM execs
`ninja` by name) — then run with CUDA graphs, no `--enforce-eager`.
- **Don't force HF offline.** With the model cached, bfcl runs fine online
(~7 req/s measured); a one-off slow run is usually a transient HF hiccup, not
a systematic per-request throttle. `run_ab.py` does **not** set
`HF_HUB_OFFLINE`; set it yourself only for air-gapped boxes. No HF token is
needed for public models.
- **New models need a bfcl handler.** bfcl-eval pins a fixed model list; a
brand-new SKU (e.g. `Qwen/Qwen3.6-27B`) isn't in it, so
`bfcl generate --model <id>-FC` fails with "Unknown model_name". Run
`register_bfcl_model.py --model-id <id>` first.
- **SMG auto model→parser mapping lags new SKUs.** SMG's factory doesn't yet map
`Qwen3.6*` (it falls back to the JSON `qwen` parser, wrong for the XML format),
so pass `--tool-call-parser qwen_xml` explicitly. Adding a `Qwen3.6*`→`qwen_xml`
mapping to `crates/tool_parser` is a good follow-up.
- **Use the `-FC` handler.** `Qwen/Qwen3-4B-Instruct-2507-FC`, not the bare name
(which is prompt mode and bypasses the server parser).
- **`bfcl generate --skip-server-setup`** points at `LOCAL_SERVER_ENDPOINT` /
`LOCAL_SERVER_PORT`. (Custom full base_urls behind a proxy are still rigid —
gorilla issue #1280.)
- **`EngineDeadError` on startup is usually the missing-`ninja` issue above**
(the engine dies around CUDA-graph capture / first compile). Install `ninja`
and put it on `PATH` rather than falling back to `--enforce-eager`.
- **`bfcl-eval` needs `soundfile`.** Its Qwen handler imports `qwen_agent` → `soundfile`; without it `bfcl --help` itself crashes. `pip install soundfile`.
- **Cap the context.** Qwen3-4B defaults to a 256K `max_model_len` → ~36 GiB KV cache → engine init OOM. Pass `--max-model-len 16384` (the launch helper defaults to this); use the **same** value on both arms.
- **Install `ninja` in the vLLM env (do NOT reach for `--enforce-eager`).** vLLM's torch.compile / CUDA-graph path shells out to `ninja` to build kernels (required for newer archs like Qwen3.6's `qwen3_5`); if it's missing the engine dies with `No such file or directory: 'ninja'`. `--enforce-eager` only *hides* this by skipping compilation (slower). Real fix: `pip install ninja` in the vLLM env **and put its bin on `PATH`** (vLLM execs `ninja` by name) — then run with CUDA graphs, no `--enforce-eager`.
- **Don't force HF offline.** With the model cached, bfcl runs fine online (~7 req/s measured); a one-off slow run is usually a transient HF hiccup, not a systematic per-request throttle. `run_ab.py` does **not** set `HF_HUB_OFFLINE`; set it yourself only for air-gapped boxes. No HF token is needed for public models.
- **New models need a bfcl handler.** bfcl-eval pins a fixed model list; a brand-new SKU (e.g. `Qwen/Qwen3.6-27B`) isn't in it, so `bfcl generate --model <id>-FC` fails with "Unknown model_name". Run `register_bfcl_model.py --model-id <id>` first.
- **SMG auto model→parser mapping lags new SKUs.** SMG's factory doesn't yet map `Qwen3.6*` (it falls back to the JSON `qwen` parser, wrong for the XML format), so pass `--tool-call-parser qwen_xml` explicitly. Adding a `Qwen3.6*`→`qwen_xml` mapping to `crates/tool_parser` is a good follow-up.
- **Use the `-FC` handler.** `Qwen/Qwen3-4B-Instruct-2507-FC`, not the bare name (which is prompt mode and bypasses the server parser).
- **`bfcl generate --skip-server-setup`** points at `LOCAL_SERVER_ENDPOINT` / `LOCAL_SERVER_PORT`. (Custom full base_urls behind a proxy are still rigid — gorilla issue #1280.)
- **`EngineDeadError` on startup is usually the missing-`ninja` issue above** (the engine dies around CUDA-graph capture / first compile). Install `ninja` and put it on `PATH` rather than falling back to `--enforce-eager`.

## Validation status — ran end-to-end ✅

Run on a dev H100 box, **`Qwen/Qwen3.6-27B` at TP=2** (one arm per GPU pair),
the **full `non_live` set** (1390 cases), FC mode, temp 0.001 — **no
`--enforce-eager`, online HF** (the clean config, after `pip install ninja`):
Run on a dev H100 box, **`Qwen/Qwen3.6-27B` at TP=2** (one arm per GPU pair), the **full `non_live` set** (1390 cases), FC mode, temp 0.001 — **no `--enforce-eager`, online HF** (the clean config, after `pip install ninja`):

| category | pure vLLM (`qwen3_xml`) | SMG → vLLM gRPC (`qwen_xml`) | Δ |
|---|---|---|---|
Expand All @@ -143,22 +99,6 @@ the **full `non_live` set** (1390 cases), FC mode, temp 0.001 — **no
| irrelevance | 84.58 | 84.58 | 0.00 |
| **overall (unweighted)** | **83.87** | **84.05** | **+0.18** |

So SMG's Rust frontend is **at parity** with vLLM's native parser across the
full non-live set — marginally ahead overall (+0.18pp), and never worse than
−0.25pp on any category. Both arms reasoning-parse `<think>` into
`reasoning_content` and emit native `tool_calls` (FC confirmed end to end). The
low java/js numbers are the *model's* non-Python ability (identical on both
arms) — confirming the A/B isolates the frontend, not model quality. (An earlier
`simple_python`-only run on Qwen3-4B gave 95.50 vs 95.25, same parity story.)

Scope note: this is **non-live only** — the reproducible AST categories, no
live/internet-dependent data. PR-time uses the cheap CPU Track-A
parser-conformance gate (`crates/parser_conformance`); the nightly runs this
full non-live A/B. Scale to multiple runs × the five target models for tight
confidence intervals.

**On the two "fixes" from the first cut:** neither `--enforce-eager` nor
`HF_HUB_OFFLINE` is actually needed. The crash was a missing `ninja` (install it
in the vLLM env + PATH → CUDA graphs work); the slowness was a transient
unauthenticated-HF hiccup (cached + online runs at ~7 req/s). Both band-aids
have been removed from the defaults.
So SMG's Rust frontend is **at parity** with vLLM's native parser across the full non-live set — marginally ahead overall (+0.18pp), and never worse than −0.25pp on any category. Both arms reasoning-parse `<think>` into `reasoning_content` and emit native `tool_calls` (FC confirmed end to end). The low java/js numbers are the *model's* non-Python ability (identical on both arms) — confirming the A/B isolates the frontend, not model quality. (An earlier `simple_python`-only run on Qwen3-4B gave 95.50 vs 95.25, same parity story.)

Scope note: the table above is the `non_live` slice. The **nightly** runs the broader reproducible set — **`non_live` + `live` + `multi_turn`** (17 categories; `live` is real-user data, `multi_turn` is state-based simulation — both static, no internet). It excludes the agentic/executable categories (`web_search_*`, `memory_*`, `exec_*`) that need web-search/memory/sandbox infra. **PRs** that touch the pipeline run only a quick non-live sanity subset (`simple_python,irrelevance`); per-PR correctness is the cheap CPU Track-A parser-conformance gate (`crates/parser_conformance`). Scale to multiple runs × the five target models for tight confidence intervals.
Loading