feat(tasks): add BFCL v3 — single-turn function calling, both protocols - #143
Merged
Conversation
`function_tools` is already a capability key, and passing `tools` projects an intent for it, but `TaskRequirements` had no slot to demand it -- so a task whose whole premise is the tools API could only fail at its first request, after a result directory existed. Add the fourth member of the existing capability-declaration set; both validation paths pick it up unchanged. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Apache-2.0 upstream, so the grader is copied rather than reimplemented. The only edit is rewriting `bfcl_eval.*` imports to relative ones; each module's docstring records upstream's blob SHA and that deviation, and a sha256 test pins the files as committed so a later edit cannot widen it quietly. `ast_checker` reads a `MODEL_CONFIG_MAPPING` global to answer one question -- may a function name contain a dot on this transport. That is a property of the protocol, not of a model, so sieval supplies the global instead of carrying upstream's 2044-line model registry. Co-Authored-By: Claude Opus 5 <[email protected]>
Review found that ast_checker.py's import-rewrite boilerplate had been copy-pasted into type_mappings.py, java_type_converter.py, and js_type_converter.py, where it doesn't hold: type_mappings.py has no imports at all (and now says so without self-contradiction), and each type_convertor file rewrites exactly one import at one level of relative nesting (`..type_mappings`), not the three-import set ast_checker.py actually has. The audit design in this task depends on these docstrings being followed literally to re-derive the copy, so a wrong one is a real risk, not a wording nit. Also drops _tables.py's dangling reference to a nonexistent `_base.py`, stating directly that LANGUAGE_BY_CATEGORY feeds ast_checker's type conversion rather than the AST parser's language. No vendored code changed -- only the three docstrings and one comment. sha256 hashes for type_mappings.py, java_type_converter.py, and js_type_converter.py are updated in the same commit since the docstring is part of the pinned file. Co-Authored-By: Claude Opus 5 <[email protected]>
`ast_parse` dispatches the java and javascript categories to tree-sitter parsers rather than Python's `ast`, and the first vendoring pass extracted `ast_parse` without the two imports that reach them -- leaving 150 of the 1390 non-live samples raising NameError at decode time on the prompt protocol. Both files are copied verbatim; neither needed an import rewrite. The three tree-sitter pins are upstream's own. 0.21.3 predates the 0.22 API break, so it is pinned rather than floored, and CI installs the group so the path cannot rot unimported. Co-Authored-By: Claude Opus 5 <[email protected]>
One pinned HF snapshot, two registered datasets split on the group boundary upstream scores separately. Three categories ship no gold, because the correct outcome there is about whether a call is produced, not which one. Gold joins on the universal index -- the `<index>` of `<category>_<index>[-<sub>-<sub>]` -- because that is upstream's key: its runner sorts both files on it, discarding the sub-indices, then pairs them positionally. One row depends on it. The prompt file's `live_multiple_1052-79-0` is graded against the gold file's `live_multiple_1052-279-0`; joining on the whole `id` would drop it and score 1052 of live_multiple's 1053 rows, publishing a column upstream never did. The key's uniqueness is asserted on every file that is joined, which is what makes an index join and upstream's positional join the same pairing -- and is stricter than upstream, which only checks the two files are the same length. It is not asserted on the goldless three, which are never joined: `live_relevance` ships one row twice, 18 rows over 17 distinct ids, and upstream scores all 18. Per-category row counts are asserted on load. The revision pin already stops a silent re-upload -- the assertion is what turns bumping the pin into a visible failure instead of a quietly rescored column. Co-Authored-By: Claude Opus 5 <[email protected]>
`TaskRequirements.function_tools` lets a task declare it needs the tools API, but the reply's structured calls never reached the task: the IR `Response` carries `tool_calls` and `response_to_model_output` rebuilt `ModelOutput` field-by-field without them, so the declaration was unsatisfiable by construction. No task read them, so nothing caught it. Forward the tuple and default the field to None. Additive: every existing construction site omits it and keeps its current behaviour. Co-Authored-By: Claude Opus 5 <[email protected]>
…istry `sieval/community/bfcl_v3/__init__.py` eagerly imported `.parser`, the one module in that package reaching tree-sitter, which lives in the optional `bfcl-v3` extra. Both BFCL v3 dataset modules import the package for two pure-data tables, and `import_all_datasets()` imports every dataset submodule in a bare loop with no per-module `except` -- so on any install without the extra, `sieval dataset list` and `sieval dataset download <anything>` died with `ModuleNotFoundError: tree_sitter`, for every dataset in the repo. `ast_parse` now resolves through a module `__getattr__`; of the eight modules in the package only `parser.py` reaches tree-sitter, so deferring it unblocks the rest. Nothing at the call sites changes. CI cannot see this: `INSTALL_GROUPS` carries `-G bfcl-v3` on purpose, to gate the Java/JavaScript decode path. The regression test therefore runs a fresh interpreter with `tree_sitter*` blocked at `sys.meta_path` -- an in-process check would pass with the eager import restored. Co-Authored-By: Claude Opus 5 <[email protected]>
Every test loaded exactly one category, so nothing in the file could see a bug that only shows up across categories. Three reverse mutations of the loader left all of them green: hoisting the per-category gold table above the loop (which hands `irrelevance_0` the gold of `simple_0`, since both key on universal index 0), resetting the row accumulator inside the loop (1390 rows become 240), and storing `function` raw instead of as the json string the sample TypedDict declares. One test loading two categories at once, one of them goldless, now kills all three. The uniqueness guards were covered only jointly: the fixture repeated the index in both files, so either `_index_rows` call satisfied the `pytest.raises` and deleting either guard alone stayed green. Split into two tests, each repeating the index in one file, each matching on the message's own file label. The file also had no module docstring, so it could not carry the AI attribution line the repo requires. Co-Authored-By: Claude Opus 5 <[email protected]>
…mport A module `__getattr__` types *every* attribute of the package as its own return type, so after ba18ad51 `ast_parse` resolved to `object` and its four call sites in `tests/unit/community/bfcl_v3/test_source_parsers.py` each read as `call-non-callable`. An explicitly declared symbol takes precedence over the fallback, so a `TYPE_CHECKING` import restores the real signature. `TYPE_CHECKING` is False at runtime, so the deferral is untouched: the out-of-process regression test, which imports the whole dataset registry with `tree_sitter*` blocked at `sys.meta_path`, still passes. This is `typing.TYPE_CHECKING`, not `from __future__ import annotations`. Co-Authored-By: Claude Opus 5 <[email protected]>
…ollide The loader claimed to be strictly stricter than upstream, which asserts `len(prompt) == len(possible_answer)`. It was not: the prompt count was checked against the category table and the gold count was not checked at all, so a re-upload that grew only the `possible_answer/` file passed every guard -- the join is by index, so the extra rows are simply never looked up. The gold count is now asserted against the same table, for the joined categories only (the goldless three have no gold file). `_index_rows` reported `len(rows) - len(by_index)` -- excess rows, not colliding keys, so three rows on one index read as "2 rows sharing an index" -- and named neither the index nor a single id, while the gold-miss guard 35 lines below prints three example ids. It now reports the colliding indices with the ids that collide on each. `_universal_index` raised a bare `ValueError: invalid literal for int()` on an id like `simple_0a`, with no category, row or file in it, on exactly the scenario the rest of this module's messages are written for: a pin bump that moves the id shape while the counts hold. It raises the same named shape as its neighbours now. Also marks the ignored `**kwargs` on both `load` overrides deliberate, as `spider.py` does. Co-Authored-By: Claude Opus 5 <[email protected]>
One generic base carries preprocess/infer/postprocess/feedback; two axis classes differ only in how schemas reach the model and how calls are read back, which is exactly what upstream's (FC) and (Prompt) columns differ in. The `underscore_to_dot` flag travels as a call argument rather than a module global, because grading runs in a worker process -- a global set in the parent never arrives, and getting it wrong grades an FC run under the Prompt convention and yields a plausible score instead of an error. Co-Authored-By: Claude Opus 5 <[email protected]>
The comment said the extra is reached "only by the Prompt protocol on the `java` and `javascript` categories". The first half is right; the second is not. `parser.py` imports both source parsers at module scope and each imports `tree_sitter` at its own module scope, with no guard, so touching `ast_parse` at all needs the extra -- a Python row included. Caught by blocking `tree_sitter` with a `find_spec` meta-path finder: importing the task package still succeeds, but calling `_decode` on a Python row raises `ModuleNotFoundError` exactly as a JavaScript row does. Comment only. No dependency changed. Co-Authored-By: Claude Opus 5 <[email protected]>
…g it A reverse mutation replacing the `gold_json is None` raise with `return False` left all 18 tests passing, and no preflight check covers it either -- the two adjacent ones, `check_record_key_access` and `check_reference_kind`, both pass under that mutation. The guarantee is a rule of its own (a value-reference task that finds no gold raises), so it is worth a test of its own: scored `False`, a dataset fault is charged to the model, and under `DENOMINATOR_REQUESTED` the headline cannot tell the two apart. Co-Authored-By: Claude Opus 5 <[email protected]>
Non-live is an unweighted mean of five, with `simple_ast` nested inside it; live is a sample-count-weighted mean of six. Both go through the vendored aggregation functions rather than being re-derived here. The split also decides the intervals. A weighted mean is algebraically the pooled rate over its rows, so live's two rollups are genuine per-sample rates and carry one. An unweighted mean of rates is not -- javascript's 50 rows weigh the same as simple's 400 -- so non-live's three publish none rather than bracketing a statistic other than the one printed beside them. Per-sample values are 0/1, not 0/100: the estimators read `sum(values) / denominator` as a probability, so percentage-point values make every rate above one point read as a saturated set and publish an interval bracketing 100. The rate is scaled to percentage points once, in `cell`, so the published number and its bounds share their units. Co-Authored-By: Claude Opus 5 <[email protected]>
Two groups upstream scores separately, each under both published protocols. The unqualified name is the prompting protocol: every model on the leaderboard has a Prompt row, while FC exists only where the provider exposes a tools API -- and FC gates on a capability, so giving it the default name would make the default the narrow one. Every stage lives in the already-shared `_base.py`, so each leaf is its metadata plus a base pair: a protocol mixin and a group base. The family test asserts both halves separately, because they fail differently -- a leaf on the wrong group base still binds the right dataset and still demands the right capability, and would publish an unweighted mean of category rates over live rows under a live-sounding score key. The FC leaves' capability refusal is `Task.__init__`'s, reached at prelaunch reconciliation before a request is made; static schema validation does not import tasks and so does not see it. Asserted rather than described, since no implemented chat dialect denies the capability today and the claim would otherwise be unfalsifiable. All four ship experimental: the licence does not force it, but nothing has been aligned against a published BFCL number yet. Co-Authored-By: Claude Opus 5 <[email protected]>
…eaves The Prompt protocol decodes replies with `ast_parse`, whose module imports both tree-sitter source parsers at module scope. The import therefore runs before the per-language dispatch does, so the extra is needed for every category rather than just `java` and `javascript` -- including the live group, whose categories are all Python. The FC leaves parse no source text and need nothing, so they keep `deps_group=None`. Without this, the readiness probe never inspects the task-deps axis (`extras_unsatisfied` is only consulted when a `deps_group` is declared) and reports `yes` on a base install. Because the import is deferred into `_decode`, the run then bills for inference before every sample dies at postprocess. `deps_group` is a frozen field on `TaskMeta`, so declaring it after these tasks ship would be a schema-contract change rather than an edit. The test asserts the mapping in both directions, since both faults are silent in production: dropping it from a Prompt leaf buys the wasted run above, and adding it to an FC leaf leaves that leaf reporting NOT ready -- unrunnable over a dependency it never loads. Co-Authored-By: Claude Opus 5 <[email protected]>
Replaying each row's own possible_answer must grade correct, which separates
"is the port wired up right" from "is the model good" -- a question no model
run can answer. 2344 of 2351 Python rows do.
The replay reads the function schema, not just gold: a `""` among a
parameter's accepted values licenses omitting it, but only where the schema
does not mark it required, and gold sometimes offers a parameter the schema
never declares. Gold also nests alternatives through both dicts and lists.
The 7 residuals are all upstream gold contradicting upstream's own schema and
are asserted BY NAME, so a snapshot that fixes one fails here rather than
passing quietly:
simple_363 gold calls it find_closest, the schema calls it
restaurant_search.find_closest
live_simple_106-63-0 gold offers "" (omit) for a required parameter
live_simple_112-68-0 same
live_multiple_507-149-4 same
live_multiple_964-207-0 same
live_multiple_862-181-3 gold names a parameter the schema never declares
live_multiple_1038-265-0 an optional parameter with no default, which the
checker then demands
Java and JavaScript are excluded: the checker requires their arguments as raw
source text for the type converters to parse, while gold holds native values,
so a replay would have to serialize Java and JS inside a test.
Co-Authored-By: Claude Opus 5 <[email protected]>
_fc earns a vocabulary row because it is a different axis from _parse: it changes which model capability is exercised, not how output is extracted, so it changes which models can run the task at all. Co-Authored-By: Claude Opus 5 <[email protected]>
…not lines The relative-import rule reached into `sieval/community/`, which pre-commit has always excluded. That was inert while every vendored relative import was a bare `from . import x`; a drop using `from ..x import y` made it bite, and the only fix the checker offered was to edit files kept byte-identical to upstream. The exemption is scoped to that one rule, inside `_check_relative_scope` -- not to the file, and not to the wrapper's file list. Both of those also disable the private-access and layer rules over all of community/, which pre-commit does not check at all and which we want to keep. Separately, the preflight wrapper reported `len(stderr lines)` as the violation count. Vendored files emit SyntaxWarning text on every run, so two real violations were reported as twenty-six. Co-Authored-By: Claude Opus 5 <[email protected]>
Both comments added with the exemption claimed it preserves "the layer, sub-package and private-module rules" over `sieval/community/`. Only the third of those binds there: `FORBIDDEN` and `FORBIDDEN_SUBPACKAGE` have no `community` entry, so a vendored file may import across layers freely and always could. The decision the comments defend is unchanged and still right -- hoisting the exemption into `_check_file` or the wrapper would drop private-module protection over a tree pre-commit does not check at all. But a reader weighing a wider exemption against three rules would be weighing it against two that cost nothing, and would reasonably conclude the guard is over-engineered. Name the one rule instead. Co-Authored-By: Claude Opus 5 <[email protected]>
…protocols Upstream runs every function schema through `func_doc_language_specific_pre_processing` before it reaches the model, on both the prompting and the native-function-calling path, and merges the schema block into the row's own system turn via `system_prompt_pre_processing_chat_model`. The port did neither, so what the model saw diverged from what upstream sends on 3637 of 3641 rows -- the four exceptions being live_irrelevance rows carrying an empty schema list, where both renderings are the string `[]`. Two concrete divergences: * the 150 Java and JavaScript rows reached the model with their real parameter types (`array`, `integer`, `object`, ...) and no language hint, where upstream restates every one as `string` and appends "Note that the provided function is in Java 8 SDK syntax." The model was being asked for Java source against a schema that never said so. * the 92 live rows that open with their own system turn received a second prepended one. Two system turns is a shape every provider accepts and answers, so the run completed and only the score was wrong. Grading is deliberately untouched. Upstream's `ast_file_runner` reads `function` back out of the dataset and hands it to `ast_checker` unprocessed, so `preprocess` still stores the raw schema for `feedback` -- verified byte-identical on all 3641 rows under both protocols, and the gold-replay anchor still grades 2351/2351. The two vendored helpers rewrite their arguments in place, so the schema list is re-parsed per call and the question turns are copied; without the copy all 92 system-turn rows corrupt the stored sample on the second pass. Co-Authored-By: Claude Opus 5 <[email protected]>
…output
`resolve_ast_by_type`'s `ast.BinOp` branch resolved a decoded call's keyword
argument with `eval(ast.unparse(value))`. That node is the model's reply, so a
reply carrying `f(x=__import__('os').system('...') + 0)` parses to a `BinOp`,
the decoder runs the payload, and it returns an ordinary-looking number. The
sample grades, the score is plausible, and nothing in the run looks unusual
afterwards. Established by side effect rather than by inspection: a probe
payload wrote a marker file, and the file appeared.
`_safe_eval.safe_eval` walks the node instead of unparsing it, so there is no
string and no namespace to escape from, and it admits only literals, their
containers, and the arithmetic/bitwise operators. Every expression upstream's
`eval` computes *without* executing something is computed here to the same
value: 19/19 benign expressions agree, and the 18 fidelity cases in the suite
pass under upstream's own `eval` too, so they encode upstream's values rather
than the port's.
It also refuses two shapes that do not execute. An f-string, where
`ast.literal_eval` draws the same line. And an expression whose cost is
unbounded -- decoding runs inline on the session's event loop with no timeout
around it (the grade timeout is in the `feedback` worker, a stage later), so
`f(x=9**9**9)` would not return a wrong answer, it would stall every other
sample in the run. Restoring upstream's `eval` makes that case run past a 40 s
kill; the guarded version returns in under 0.1 s.
Evidence no bound binds on the pinned data: all 2351 gradeable Python rows
decode, 0 `BinOp` nodes reach the branch at all, and the largest integer
literal across the gold answers is 13 digits against a 2**20 cap. Reverse
mutation confirms the tests discriminate -- restoring upstream's `eval` fails
exactly the 10 safety tests and leaves every fidelity test passing.
The `ast.Lambda` branch keeps upstream's second `eval` byte-identical, because
it cannot reach it: `Lambda.body` is a single expression node, so
`value.body[0]` raises `TypeError` while the argument is still being built.
Hardening a line that cannot execute would be a deviation bought for nothing.
A test pins the `TypeError` and so fails if someone later "repairs" the
subscript, which would install a live `eval` over model output.
The guard is held outside `parser.py` because that file is vendored: inlining
it would enlarge a diff that exists to be compared against upstream, and would
put a security boundary inside a file no linter runs over. `community/`'s
CLAUDE.md records the hand-lint command, since the package-wide exclusions
cover it.
Separately, `preprocess.py` had been vendored without being added to the
identity map, so a vendored file went unprotected by the drift test for a
commit. Added, along with a test that each file's recorded provenance is
resolvable, and a note that `Upstream path:` is relative to
`berkeley-function-call-leaderboard/` rather than the repo root. All 12 blob
SHAs re-verified against gorilla@ea13468e, 0 mismatches.
Co-Authored-By: Claude Opus 5 <[email protected]>
…onment `_decode` defers its import of the vendored parser, because tree-sitter lives behind the optional `bfcl-v3` extra and importing it eagerly makes that extra a hard requirement. The deferred import therefore raises from inside the `try` whose `except Exception` exists to turn an undecodable reply into a score. An environment without the extra was being graded, not reported: every row of the run scores wrong, `fails` stays 0, and the result is indistinguishable from a model that cannot call a function. `ImportError` now propagates, which costs the run and names the cause. Two tests pin the split -- one that an undecodable reply is still scored rather than raised, and one that a missing parser dependency propagates. Separately, two docstrings justified `underscore_to_dot` being a call argument with "grading runs in a worker process, so a global set in the parent never arrives." That is false: `run_cpu_bound` falls back to running inline on the event loop whenever no worker pool is available, so the global would arrive fine. The argument-passing is still right, but for a different reason -- `grade_single_turn` writes the global the vendored checker reads and consumes it with no `await` in between, which is what makes the pair atomic. A caller that set the global and then awaited would let another sample's grade land between the set and the use. A reader who checked the stated reason would find it wrong and could reasonably conclude the constraint was imaginary, so the justification is corrected to the one that holds. Co-Authored-By: Claude Opus 5 <[email protected]>
…grader `BFCL_V3_SHARED_NOTES` said the vendored parsers were "vendored verbatim ... only `bfcl_eval.*` imports were rewritten." That stopped being true one commit ago, when `parser.py`'s `ast.BinOp` branch was changed to walk the argument's AST instead of `eval`-ing it. `parser.py`'s own docstring records the deviation; the notes did not, and the notes are the copy that ships -- they reach all four tasks' `reference_impl` and are embedded verbatim in `sieval/meta/index.json`, so an archived run's `meta.json` asserted an unmodified grading path. That claim is exactly the one a reader consults before arguing a divergence, so a false "verbatim" is worse than no note at all: it answers the question wrongly instead of prompting a look. The text now names the deviation, what it computes instead, and that the FC protocol never reaches it -- FC parses no source text, so two of the four tasks carry the note for a path they do not execute. `meta/index.json` regenerated; the delta is the four BFCL entries and nothing else. Co-Authored-By: Claude Opus 5 <[email protected]>
…ng as a path The shared notes named the four skipped groups as `exec_*/rest/sql/chatable`, which carries a `/rest/sql` substring. `scripts/sanitize.sh`'s config scan flags any absolute-looking path whose first segment is not a system root, and the `*` before the first slash clears its lookbehind, so the regex matched -- on all four BFCL entries of `sieval/meta/index.json`, which embeds these notes verbatim. CI's sanitize job failed on those four lines. Reworded to a comma list instead of adding an ALLOWLIST entry: the pattern is a security gate, an exemption for it would have to be scoped by file, and the four categories were never a path to begin with -- they are BFCL test-file group names. `sieval/meta/index.json` regenerated; the delta is those four `notes` strings and nothing else. Co-Authored-By: Claude Opus 5 <[email protected]>
…bound string `%` Two review findings, plus two doc nits. `..type_mappings` in the two vendored type converters was the only thing that needed `sieval/community/` exempted from the cross-package relative-import rule. Upstream's own import was absolute (`bfcl_eval.constants.type_mappings`) and a vendored copy cannot keep it, so that line was already rewritten -- spelling the replacement absolutely costs the same one line and satisfies the rule as it stands. The exemption is reverted: preflight is the only surface that checks `community/` at all, since pre-commit's global `exclude` skips it, so a carve-out there would have taken private-module protection with it for every future vendored drop. The mirror tests now pin the opposite decision. `_safe_eval` screened `Pow`, `LShift` and `Mult` but not `Mod`, so `%` formatting on a string was caught only by the post-hoc size check -- after the allocation. `"%.400000000f" % 1.0` asks for a 400MB string from two tiny operands, which is the class the guard exists to close; a precision field sets the result size independently of both operands, so there is nothing to measure and the shape is refused outright. Integer `%` is bounded by its right operand and stays admissible. Not a regression -- upstream's `eval` has the same exposure -- and still score-neutral, since no gold row reaches `safe_eval`. Also: the dataset docstrings pinned the harness to the `v1.3` tag while `_base.py` pinned the commit; both now carry the SHA. And `tree_sitter` is spelled `tree-sitter` to match its two siblings, re-locked with `--update-reuse` (content_hash only, no version drift). Verified: 7575 unit tests pass, 25/25 preflight checks PASS, ruff and ty clean, and the gold replay still walks 2351 rows failing only the 7 documented upstream gold defects. The new `%` test is reverse-mutation checked -- its 22-char case, which a post-hoc size check admits, returns a value with the guard deleted. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…ed rollouts BFCL publishes an evaluation archive beside its leaderboard (HuanzhiMao/BFCL-Result): per model, per protocol, per category, both the model's recorded replies AND upstream's own per-row verdicts. Replaying gpt-4.1-2025-04-14's rollouts through our grader reproduces every published cell exactly and agrees with upstream's verdict on every row: protocol group rows agreement Prompt non-live 1390 1390/1390 Prompt live 2251 2251/2251 FC non-live 1390 1390/1390 FC live 2251 2251/2251 All 26 per-category accuracies match to the digit, so the four leaves now record their own published numbers the way the other reproduction tasks do, and ship `stable`. The sentence they carried -- "experimental until an alignment run against a published BFCL v3 number lands" -- was the only thing blocking it, and it is now false. This is a strictly stronger anchor than the gold replay, which feeds gold back and so only ever exercises the correct path: it never sees a wrong answer, a decode failure, or either protocol's decoder. The new acceptance test drives the production `_decode` of both mixins over output that is 9% wrong. `java` at 64% Prompt / 61% FC is the point -- a third of those rows are wrong and we have to agree about which third. It also pins the two rollups against their own documented claims: live's weighted mean reconstructs the pooled rate over all 2251 rows, and non-live's mean-of-means is asserted NOT to equal the pooled rate, so a "fix" toward pooling fails loudly. Both mutations it is meant to catch were checked by reverse-mutation: flipping FC's `UNDERSCORE_TO_DOT` (the silent cross-wiring `_base` warns about) and forcing the Prompt decoder to language="Python" (so Java/JavaScript stop reaching tree-sitter) each fail the test. What the anchor does NOT measure is stated in both the test and the shipped notes: the replies are upstream's, so nothing here proves our prompt is upstream's. That rests on the two model-facing helpers being byte-identical to upstream and running on a row-for-row verified snapshot -- a construction argument, not a measurement. The archive is ~6MB, so it is staged on demand rather than vendored, and the test skips with a reason naming the layout. No file hashes are pinned: every score file's header is asserted against the pinned counts, so a swapped archive fails, and a doctored result file fails the comparison it feeds. Verified: 7575 unit tests pass, 9 acceptance tests pass with the archive staged and skip cleanly without it, 25/25 preflight checks PASS (meta/index.json regenerated), ruff and ty clean. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…eady say
The port shipped 1751 lines of comment and docstring against 3689 of code, and
the bulk of the excess was one fact stated in several places rather than any
single essay. Net -140 lines over 18 files, with no fact dropped.
Give each fact one owner:
- The four leaves each said the same thing twice -- a module docstring and a
`reference_impl.notes` restating the headline formula and the interval policy
in different words. `notes` keeps them, because it is what reaches
`meta/index.json` and so the only copy a reader without the source ever sees;
the docstrings now say what the task is and stop.
- `underscore_to_dot` was explained four times (`_base` module docstring,
`grade_single_turn`, both `_fc` leaves). `_base` owns the mechanism since the
flag lives there; `grade_single_turn` owns the atomicity argument since it is
that function's shape being justified, and it absorbs the 734-row number that
quantifies the risk.
- Why the unqualified name goes to Prompt was argued in every leaf and again in
`tasks/CLAUDE.md` §Variants, which is the naming vocabulary's actual home.
- The `source_parser/` rename was explained three times, at length.
Also drops a private reference: `tool_convert.py` cited "Step 3's whole-file
modules", a step number from an internal implementation plan that means nothing
in a public checkout.
Kept deliberately: every non-derivable fact, including the two that read as
padding and are not -- `_safe_eval`'s calibration evidence (13-digit largest
gold literal across 2351 rows) and the ImportError/`.get("prediction")`
rationales, both of which describe silent failures. Test docstrings are left
alone after sampling them: at 8-19 lines they are dense rather than padded, and
the longest states two production failure modes a reader cannot infer.
Prose-only, and checked rather than asserted: with docstrings stripped and string
constants normalized, every touched file's AST is identical to its previous
version. The only literals that move are one `notes` per leaf, the shared notes
block, and the six vendored digests whose docstrings were trimmed.
Verified: 7575 unit tests pass, 9 acceptance tests pass, 25/25 preflight checks
PASS (meta/index.json regenerated), ruff and ty clean.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type
Summary
Adds Berkeley Function Calling Leaderboard (BFCL) v3, single-turn — 4 tasks over 2 datasets, covering both of upstream's protocols.
bfcl_v3_non_live(1,390 rows / 7 categories) andbfcl_v3_live(2,251 rows / 6 categories), both pinned tohf:gorilla-llm/Berkeley-Function-Calling-Leaderboard@61fc0608(Apache-2.0). Per-category counts verified against the snapshot, all 13 exact.bfcl_v3_non_live_0shot_gen,bfcl_v3_live_0shot_gen(upstream's Prompt protocol — the model emits text that a vendored decoder parses), and the_fcpair (upstream's native function-calling protocol)._fcmarks a capability axis — which model interface is exercised — deliberately different from the_parseprompt/extractor axis.bfcl_v3_non_live_0shot_genmeans what an upstream reader expects;_fcis the marked variant.stable: replaying upstream's own released rollouts reproduces every published cell exactly on 7282/7282 rows (see Manual).TaskRequirements.function_toolsso a task can declare the capability rather than have it inferred; the declaration was unsatisfiable by construction untiltool_callswas forwarded through the legacy model bridge.Related Issues
Refs #47 — core now has one declared capability constraint (
TaskRequirements.function_tools) enforced at readiness, rather than the silent cross-group failure that RFC describes. This PR does not implement the RFC.Test Plan
Automated
ruff check && ruff format --check)ty check)pytest tests/unit— 7570 passed)python scripts/check_preflight.py— all 25 checks PASSManual
Score comparison: exact, on every published cell. BFCL publishes an evaluation archive beside its leaderboard (
HuanzhiMao/BFCL-Result,2025-06-14snapshot) carrying, per model and per protocol, both the model's recorded replies and upstream's own per-row verdicts. Replayinggpt-4.1-2025-04-14's rollouts through this port reproduces all 26 published category accuracies to the digit and agrees with upstream's verdict on every row — no model spend, and a tighter check than a live run, which would carry sampling noise.Row-for-row agreement: Prompt 1390/1390 non-live + 2251/2251 live; FC 1390/1390 + 2251/2251. 7282/7282, zero disagreements. Pinned in
tests/acceptance/bfcl_v3/test_upstream_rollout_replay.py.This anchor is strictly stronger than the gold replay below, which feeds gold back and so only ever exercises the correct path — it never sees a wrong answer, a decode failure, or either protocol's decoder. The new test drives the production
_decodeof both mixins over output that is 9% wrong;javaat 64%/61% is the point, since a third of those rows are wrong and we have to agree about which third. It also pins both rollups against their own documented claims: live's weighted mean reconstructs the pooled rate over all 2251 rows, while non-live's mean-of-means is asserted not to equal the pooled rate, so a "fix" toward pooling fails loudly. Reverse-mutation checked: flipping FC'sUNDERSCORE_TO_DOTand forcing the Prompt decoder tolanguage="Python"each fail it.What the anchor does not measure, stated here and in the shipped notes: the replies are upstream's, so nothing here proves our prompt is upstream's prompt. That rests on
system_prompt_pre_processing_chat_modelandfunc_doc_language_specific_pre_processingbeing byte-identical to upstream (pinned bytest_identity.py) and running on a row-for-row verified snapshot — a construction argument, not a measurement.The archive is ~6MB, so it is staged on demand rather than vendored; the test skips with a reason naming the layout.
tests/acceptance/bfcl_v3/test_gold_replay.py): upstream's own gold answers are fed back as the reply and must grade correct. This anchors the harness on upstream's code run now rather than on a published number, and catches grader wiring breakage without model spend. Verified on the pinned snapshot: 2,351 gradeable Python rows walked, and the only failures are the documented upstream gold defects — any new failure fails the test as our defect, and any defect that starts passing fails it as an upstream fix that needs the set updated. Note this anchor is opt-in on a downloaded snapshot, so it does not run in CI.eval-uates model output when resolving arithmetic in an argument. The vendoredparser.pynow walks the AST instead. Two measurements on the pinned snapshot bound the deviation: replaying all 2,351 gradeable Python gold answers callssafe_evalzero times (measured with a patchedsafe_eval; a control run onf(x=1+2)proves the spy fires when the branch is taken, so the zero is a real negative, not a silent no-op), and of the gold literals that would reach the branch, the walk agrees withevalon 49/49 — 0 differing values, 0 refusals. The deviation is therefore score-neutral on this data and changes behaviour only whereevalwould execute something.f(x=9**9**9)would stall the whole run rather than one sample; the guard refuses instead.ast.Lambdabranch was left byte-identical to upstream, with aTypeError-pinning test.ast.Lambda.bodyis a singleexpr, so upstream's secondevalthere is unreachable —value.body[0]raises first. Rewriting it would have been a change with no reachable effect.Checklist
Required (all PRs)
type(scope): description)AI-Generated Code - <model> (<provider>)in module docstringcore/If: New or Modified Benchmark
sieval dataset download <name>succeeds)__init__.pyIf: community/ Changes
Upstream deviation, all of it recorded in the vendored docstrings:
bfcl_eval.*imports rewritten tosieval.community.bfcl_v3.*.ast.BinOparithmetic resolved by AST walk instead ofeval(the RCE path above).Two further notes: the safety guard lives in
_safe_eval.py, outsideparser.py, so the deviation is auditable in one place; and the vendoredast.NameConstantusage emits aDeprecationWarningon every test run and will break on Python 3.14 — tracked indocs/TODO.md, not fixed here, because replacing it is a divergence from the pinned upstream file with no behavioural effect today.If: Breaking Change
If: New Dependency
New extra
bfcl-v3:tree_sitter==0.21.3,tree-sitter-java==0.21.0,tree-sitter-javascript==0.21.4. Reached only by the Prompt protocol on thejavaandjavascriptcategories — upstream'sast_parsedispatches those two to tree-sitter rather than Python'sast. The FC protocol never parses source text, and every Live category is Python. Versions are upstream's own, and thetree_sitterpin is hard rather than a floor because 0.21.3 predates the 0.22 API break (the vendored parsers callLanguage(ptr, name)andParser.set_language(), both removed later); it has no wheel for Python 3.13 and builds from sdist. CI installs the group so that decode path is actually exercised — the cost is one small C extension per run.🤖 Generated with Claude Code