Repository navigation
feat(ci): add BFCL function calling accuracy tests to nightly and PR CI - #520
vschandramourya wants to merge 1 commit into
Conversation
Add end-to-end BFCL v3 (Berkeley Function Calling Leaderboard) test infrastructure with 1,240 test cases across 5 categories (simple, multiple, parallel, parallel_multiple, irrelevance) and integrate it into both nightly and PR CI workflows. Nightly CI: full BFCL suite (all 1,240 cases) runs on SGLang with Qwen2.5-7B, results flow into the nightly summary with per-category accuracy tables and baseline regression detection. PR CI: lighter BFCL (20 cases per category = 100 total) runs on both SGLang and vLLM via BFCL_LIMIT=20, providing fast function-calling smoke coverage on every pull request. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThis PR introduces comprehensive BFCL (Berkeley Function Calling Leaderboard) support to the project. It adds a nightly GPU-enabled benchmark workflow, BFCL data loader and evaluator modules, baseline comparison infrastructure for regression detection, a summary integration layer, and an end-to-end test suite covering simple/multiple/parallel/irrelevance function-calling scenarios. Changes
Sequence DiagramsequenceDiagram
participant Workflow as GitHub Workflow
participant DataMgr as Data Manager
participant TestRunner as Test Runner
participant Evaluator as Evaluator
participant Logger as Logger/Summary
participant Baseline as Baseline Comparison
participant Output as Output Artifacts
Workflow->>DataMgr: Download BFCL data
DataMgr->>DataMgr: Fetch from HuggingFace
Workflow->>TestRunner: Execute BFCL tests
TestRunner->>TestRunner: Load category test cases
TestRunner->>TestRunner: Convert to OpenAI tools
TestRunner->>Evaluator: Evaluate tool calls
Evaluator->>Logger: Save per-test JSON log
Evaluator->>Logger: Compute results & latencies
Logger->>Baseline: Load baseline (if exists)
Baseline->>Baseline: Compare accuracy vs baseline
Logger->>Logger: Generate summary with stats
Logger->>Output: Upload summary & logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @vschandramourya, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's testing capabilities by incorporating the Berkeley Function Calling Leaderboard (BFCL) v3. The primary goal is to establish robust, automated validation for tool-calling accuracy across different models and backends. By integrating these tests into both nightly and PR CI, the system gains continuous monitoring for potential regressions in function calling performance, ensuring the reliability and correctness of tool-use features. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Closing this PR because the branch name To fix this, please rename your branch locally, push the new branch, and open a new PR: git branch -m <new-branch-name>
git push origin -u <new-branch-name> |
|
Hi @vschandramourya, the branch Please use one of the following formats:
Allowed types:
|
|
Hi @vschandramourya, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive testing framework for function calling accuracy using the BFCL benchmark, which is a significant enhancement to the project's testing capabilities. The new infrastructure for baselining, evaluation, and logging is well-structured, and the integration into CI for both nightly and PR workflows is a great addition for regression testing. My feedback focuses on improving the robustness of the reporting scripts by adding more explicit error logging and ensuring consistency in the test result data collection.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Silently ignoring exceptions when parsing metadata.json can mask problems. If the file is corrupt, it will be treated as if it doesn't exist, leading to 'unknown' values in the summary report. This can be misleading and make debugging harder. It's better to log a warning to stderr to make these issues visible.
| except Exception: | |
| pass | |
| except Exception as e: | |
| print(f"Warning: Failed to parse BFCL metadata {meta_path}: {e}", file=sys.stderr) |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Similar to the metadata parsing, silently ignoring exceptions when parsing comparison.json can hide issues. If the file is corrupt, the baseline comparison section will be missing from the summary without any indication of why. Logging a warning would make this failure explicit and easier to debug.
| except Exception: | |
| pass | |
| except Exception as e: | |
| print(f"Warning: Failed to parse BFCL comparison data {comp_path}: {e}", file=sys.stderr) |
| save_test_log( | ||
| run_dir, | ||
| test_id=test_id, | ||
| category=category, | ||
| model=model, | ||
| parser=parser, | ||
| backend=backend, | ||
| request_payload=request_payload, | ||
| response_payload=None, | ||
| ground_truth=case.get("ground_truth", []), | ||
| actual_tool_calls=[], | ||
| passed=False, | ||
| errors=[f"API error: {exc}"], | ||
| latency_ms=latency, | ||
| ) | ||
| _all_results.append({ | ||
| "test_id": test_id, | ||
| "category": category, | ||
| "passed": False, | ||
| "errors": [f"API error: {exc}"], | ||
| "latency_ms": latency, | ||
| "finish_reason": None, | ||
| "completion_tokens": None, | ||
| "had_reasoning": False, | ||
| "log_file": f"{category}/{test_id.replace('/', '_')}_FAIL.json", | ||
| "model": model, | ||
| "backend": backend, | ||
| }) |
There was a problem hiding this comment.
The log_file path stored in _all_results for failed API calls is constructed manually and is inconsistent with the success path. In the success path, log_path.name is used, which is just the filename. In this error path, a relative path including the category is constructed. To improve consistency and reduce brittleness, you should capture the returned path from save_test_log and use its .name attribute, just like in the success case.
log_path = save_test_log(
run_dir,
test_id=test_id,
category=category,
model=model,
parser=parser,
backend=backend,
request_payload=request_payload,
response_payload=None,
ground_truth=case.get("ground_truth", []),
actual_tool_calls=[],
passed=False,
errors=[f"API error: {exc}"],
latency_ms=latency,
)
_all_results.append({
"test_id": test_id,
"category": category,
"passed": False,
"errors": [f"API error: {exc}"],
"latency_ms": latency,
"finish_reason": None,
"completion_tokens": None,
"had_reasoning": False,
"log_file": log_path.name,
"model": model,
"backend": backend,
})
CatherineSue
left a comment
There was a problem hiding this comment.
Can you split the PR into smaller ones? It is hard to review with 4k lines of changes. Thing like .yml doesn't have to be in the same PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0dd21ef20
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if isinstance(expected, list) and isinstance(actual, list): | ||
| if len(expected) == len(actual): | ||
| return all(_values_match(a, e) for a, e in zip(actual, expected)) | ||
| return False |
There was a problem hiding this comment.
Add dict-aware matching for nested BFCL arguments
_values_match never handles dictionary structures recursively, so BFCL cases whose expected values are object-shaped alternatives (for example budget/gradeDict entries in the answer files) are treated as mismatches even when the model returns correct scalar fields. Because only list recursion is implemented here, valid tool calls with nested object arguments are systematically marked as failures, which underreports BFCL accuracy.
Useful? React with 👍 / 👎.
| for i in range(check_count): | ||
| gt_entry = ground_truth[i] | ||
| actual = actual_tool_calls[i] |
There was a problem hiding this comment.
Match parallel tool calls without positional coupling
The evaluator aligns calls strictly by index (ground_truth[i] vs actual_tool_calls[i]), so two equivalent sets of calls fail whenever the model emits them in a different order. This is especially problematic for the parallel and parallel_multiple categories, where call ordering is non-semantic and backend generation order can vary, leading to false negatives in accuracy reporting.
Useful? React with 👍 / 👎.
| "https://huggingface.co/datasets/gorilla-llm/" | ||
| "Berkeley-Function-Calling-Leaderboard/resolve/main" | ||
| ) |
There was a problem hiding this comment.
Pin BFCL download source to an immutable dataset revision
The download script pulls from resolve/main, so each CI run can overwrite the checked-in BFCL fixtures with whatever is currently on the upstream default branch. Since both nightly and PR workflows invoke this script before BFCL tests, benchmark coverage and baseline comparisons become non-reproducible and can regress due to upstream dataset changes unrelated to this repository.
Useful? React with 👍 / 👎.
Summary
Add end-to-end BFCL v3 (Berkeley Function Calling Leaderboard) test infrastructure and integrate it into both nightly and PR CI workflows for tool-calling accuracy validation.
What's included
e2e_test/bfcl/): data loader, evaluator, per-test JSON logging, and HuggingFace data downloader for 1,240 test cases across 5 categories (simple, multiple, parallel, parallel_multiple, irrelevance)e2e_test/baselines/): stores accuracy baselines per model/backend, detects regressions with configurable tolerance (default ±3.0pp)e2e_test/chat_completions/test_bfcl.py): pytest classes for each category with fixture-based backend setup + standalone modeCI integration
nightly-benchmark.yml)pr-test-rust.yml)BFCL_LIMIT=20)Nightly additions
bfcl-accuracyjob on 4×H100 running all 1,240 BFCL test cases with Qwen2.5-7B-Instructnightly_summarize.pywith per-category accuracy table and baseline regression trackingsummarize-benchmarksjob updated to depend onbfcl-accuracyPR additions
BFCL_LIMIT=20added tochat-completions-sglangandchat-completions-vllmenv vars (20 cases × 5 categories = 100 tests per runtime)download_bfclmatrix flag + "Download BFCL test data" conditional stepchat-completions-*jobs — no additional GPU allocation neededNightly summary rendering
Test plan
BFCL_LIMIT=20produces exactly 20 tests per category (100 total) in PR runsbfcl-accuracyjob runs all 1,240 cases on SGLangchat-completions-sglangandchat-completions-vllmskip_for_runtimemarkerMade with Cursor
Summary by CodeRabbit
New Features
Tests