Repository navigation
feat(bfcl): nightly covers non_live + live + multi_turn - #1764
Conversation
…-live sanity) Expands the nightly BFCL A/B category set from non_live (7) to non_live + live + multi_turn (17): adds the 6 live (real-user data) and 4 multi_turn (state-based) categories — all static/reproducible, no internet. Excludes the agentic/ executable categories (web_search/memory/exec) that need external infra. - schedule/dispatch default -> 17 categories; PR sanity stays simple_python, irrelevance. - bfcl-ab timeout 120 -> 240 min (multi_turn state simulation is much slower). - README scope note updated. Signed-off-by: key4ng <rukeyang@gmail.com>
Signed-off-by: key4ng <rukeyang@gmail.com>
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughThe nightly BFCL workflow is updated to expand default test categories to include live and multi-turn entries (excluding agentic/executable), doubling the ChangesBFCL Nightly Coverage Expansion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the scripts/bfcl/README.md file to clarify the scope of nightly runs versus PR-time checks, and removes outdated troubleshooting notes. The feedback suggests a minor phrasing improvement in the README to clarify how per-PR correctness is verified.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| (`simple_python,irrelevance`); per-PR correctness is the cheap CPU Track-A | ||
| parser-conformance gate (`crates/parser_conformance`). Scale to multiple runs × |
There was a problem hiding this comment.
The phrase "per-PR correctness is the cheap CPU Track-A parser-conformance gate" contains a minor category error (correctness is a property, whereas the gate is a mechanism). It would be clearer to state that correctness is verified by or checked via the gate.
Suggested change:
(`simple_python,irrelevance`); per-PR correctness is verified by the cheap CPU Track-A
parser-conformance gate (`crates/parser_conformance`). Scale to multiple runs ×![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Straightforward CI config change — expands nightly BFCL A/B from 7 non_live categories to 17 (non_live + live + multi_turn), bumps timeout appropriately, and updates the README scope note. Category lists are consistent across the workflow_dispatch input default and CATEGORIES env fallback. PR-time sanity subset is unchanged. No issues found.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/nightly-bfcl.yml:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 05b51fc0-f759-4bd0-aa17-5f2ca9f82c53
📒 Files selected for processing (2)
.github/workflows/nightly-bfcl.ymlscripts/bfcl/README.md
| # 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') }} | ||
| steps: |
There was a problem hiding this comment.
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.
![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: a409ae42b6
ℹ️ 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".
| # 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') }} |
There was a problem hiding this comment.
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 👍 / 👎.
…aps) Signed-off-by: key4ng <rukeyang@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/bfcl/README.md (1)
104-105: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd validation-status caveat for
liveandmulti_turncategories.The scope note correctly distinguishes the nightly's expanded coverage (
non_live + live + multi_turn, 17 categories) from the validation table'snon_liveslice. However, per the PR objectives,liveandmulti_turncategories have not yet run end-to-end. Users reading this section should be advised to manually validate or observe the first nightly run before relying on results from those newly-added categories.📝 Suggested addition
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. + +⚠️ **Note:** The `live` and `multi_turn` categories are newly added to nightly coverage and have not yet been run end-to-end. Recommend a manual workflow dispatch or observing the first scheduled nightly run to validate functionality and results before relying on them.🤖 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` around lines 104 - 105, The scope note in the README.md file describes the nightly test coverage including `live` and `multi_turn` categories but lacks a validation caveat. Add a clear warning statement after mentioning these categories to advise users that `live` and `multi_turn` have not yet completed end-to-end execution, and they should manually validate results or wait for the first nightly run before relying on data from these newly-added categories. This caveat should be placed within the scope note paragraph to inform readers of the validation status before they draw conclusions from results in these categories.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@scripts/bfcl/README.md`:
- 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.
---
Outside diff comments:
In `@scripts/bfcl/README.md`:
- Around line 104-105: The scope note in the README.md file describes the
nightly test coverage including `live` and `multi_turn` categories but lacks a
validation caveat. Add a clear warning statement after mentioning these
categories to advise users that `live` and `multi_turn` have not yet completed
end-to-end execution, and they should manually validate results or wait for the
first nightly run before relying on data from these newly-added categories. This
caveat should be placed within the scope note paragraph to inform readers of the
validation status before they draw conclusions from results in these categories.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8114758b-1d88-4a78-ba19-2125af4e646a
📒 Files selected for processing (1)
scripts/bfcl/README.md
| > `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) |
There was a problem hiding this comment.
🧹 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
2 participants
Description
Problem
The nightly BFCL A/B only ran the 7
non_liveAST categories (~16 min). It didn't exerciselive(real-user function-calling data) ormulti_turn(multi-turn tool-use loops) — the categories most representative of real agentic use, and multi-turn in particular is a distinct parser/streaming code path.Solution
Expand the nightly to
non_live+live+multi_turn(17 categories). All three are static/reproducible (no internet —liveis real-user data,multi_turnis state-based simulation). Excludes the agentic/executable categories (web_search_*,memory_*,exec_*) that need web-search / memory / sandbox infra. The PR-time sanity run is unchanged (quick non-live subset).Changes
.github/workflows/nightly-bfcl.yml: schedule/dispatchCATEGORIESdefault → 17 categories (non_live + live + multi_turn);workflow_dispatchcategoriesinput default updated to match; PR-event categories unchanged (simple_python,irrelevance);bfcl-abtimeout-minutes120 → 240 (multi-turn state simulation is much slower).scripts/bfcl/README.md: nightly-scope note updated; removed a dev-history note.Test Plan
nightly-bfcl.ymlparses; 17 categories resolved; triggers = schedule + pull_request + workflow_dispatch; pre-commit green.non_liveslice is already validated end-to-end on an H100 box (per the README table: SMG at parity, +0.18pp overall).live+multi_turn. Recommend a manualworkflow_dispatch(or watching the first nightly) to shake them out before relying on the numbers —multi_turnis a new code path (multi-turn tool loops + reasoning) and the main thing to verify, plus confirming the ~240 min budget holds.Checklist
cargo +nightly fmtpasses (n/a — no Rust changes)cargo clippy --all-targets --all-features -- -D warningspasses (n/a — no Rust changes)🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores