toolcaller-v2 round 3: fix generalization regression, promote to registry - #101
Conversation
…to registry
Round 2 fixed round 1's plausible_fabrication safety regression (63.9% -> 95.5%
cp95-lower) but traded away generalization accuracy in the process (93.3% -> 81.3%
on the held-out generalization arena, missing the 85% floor). Diagnosed via a new
eval_toolcaller.py --dump-failures flag: 24/28 mismatches were the model predicting
{"decision": "unsupported", "reason_code": "no_matching_tool"} for a REAL
held-out-family tool whose full schema WAS present in available_tools -- the exact
response shape round 2's fabrication-decoy training taught, misfiring on a target
it should have called. Root cause: every round-2 training row using explicit-name-
drop phrasing ("use X to...", "call the X tool so we can...") was a decoy (correct
answer: refuse); normal synthetic-tool call scenarios never used that phrasing, so
the model learned the phrasing pattern itself as weak evidence for refusal.
Fix: explicit_call_scenarios (generate_toolcaller_v2_dataset.py, --explicit-call-only)
generates the direct counterbalance -- same explicit-name-drop phrasing as the
decoys, but targeting a real or train-pool-synthetic tool that IS in
available_tools, expected decision "call". 117 fresh captures generated (~1:1
against round 2's 120 decoys), composed into round 3's training set alongside the
unchanged v0 and v2-bulk-r2 streams (2572 train / 428 eval total).
Round 3 result, all three real eval sets: v0 regression arena 97.7% (incumbent
98.46%, within the 2pt budget), v0 holdout gauntlet 99.78% strict / 100% safety
(no regression), generalization arena 91.3% (floor 85%, up from r2's 81.3%),
plausible_fabrication safety 95.78% cp95-lower (floor 90%, essentially flat vs
r2's 95.5%) -- the fix recovered generalization without giving back the safety
gain, exactly as the diagnosis predicted.
Promotion gate fixes (foundry_promote.py / foundry_preflight.py) -- these were
previously left as deliberate config-posture gaps in round 2's config; digging in
during round 3 found two were real, fixable defects and the third needed a
genuinely different check for v2's dataset shape, not a config workaround:
- sealed_eval_hash: just needed the config to record the eval file's real current
hash (r2 never set this).
- runtime_schema_identity: foundry_preflight.py had its own hardcoded tool_schema_hash
check with no gates.tool_schema_path override support, separate from
foundry_promote.py's gate (which already supported the override) -- this is why
r2's config deliberately left tool_schema_hash null. Fixed at the root: patched
foundry_preflight.py to honor tool_schema_path the same way, verified
byte-identical preflight output on the v0 config before/after.
- frozen_group_split: v0's gate checks "did the generator run with the frozen
frac/seed," but v2 datasets are concatenated from independently lineage-split
streams, not one frac/seed split -- that check doesn't apply. Added a v2-specific
branch that directly verifies zero example_id overlap between the composed train
and eval files on disk, a real leakage check rather than a parameter-trust one.
Keyed off meta.json's isolation-field absence (the actual v0/v2 discriminator --
both shapes have a "sources" key, just differently shaped).
Round 3 promoted: all 16 gates pass, approved by hardcoreerik ("generalization
regression fixed via explicit-call counterexamples, both floors clear"), registry
entry written to training_pit/foundry/PROMOTION_REGISTRY.json. Candidate GGUF
exported (LoRA-only, matching the existing naming convention) but not yet deployed
-- that remains a separate, later manual step.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds explicit-call counterexample generation for Toolcaller v2 round 3. It defines composed dataset metadata and Foundry gates, improves schema and split validation, adds evaluation failure dumps, and records the promoted round 3 artifact. ChangesToolcaller v2 round 3 workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DatasetGenerator
participant DatasetMetadata
participant EvalToolcaller
participant FoundryPromote
participant PromotionRegistry
DatasetGenerator->>DatasetMetadata: Produce explicit-call counterexamples
DatasetMetadata->>EvalToolcaller: Supply composed evaluation dataset
EvalToolcaller->>FoundryPromote: Return evaluation results and failure diagnostics
FoundryPromote->>PromotionRegistry: Record promotion checks and approval
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
training_pit/foundry/scripts/foundry_preflight.py (1)
145-147: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSkip the sidecar hash comparison when the sidecar does not record one.
training_pit/foundry/configs/toolcaller_v2_r3.jsonsetsgates.tool_schema_hash, which activates the comparison attraining_pit/foundry/scripts/foundry_preflight.py:145. The referenced sidecartraining_pit/datasets/toolcaller_v2_r3.meta.jsonhastool_schema_hash: null, so preflight adds a SCHEMA finding andtrain_foundry.pyblocks training. Use the same pattern asfoundry_promote.py: only comparemeta["tool_schema_hash"]when it is present; rely on the already-run frozen file hash check otherwise.🤖 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 `@training_pit/foundry/scripts/foundry_preflight.py` around lines 145 - 147, Update the tool_schema_hash comparison in the preflight metadata validation so it runs only when the sidecar records a non-null hash. Preserve the existing mismatch finding when a hash is present, and otherwise rely on the frozen file hash check already performed by the preflight flow, matching the behavior in foundry_promote.py.
🧹 Nitpick comments (6)
training_pit/foundry/scripts/foundry_preflight.py (1)
137-142: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valuePrefer
gates.get("tool_schema_path")over the membership test.Line 139 uses
"tool_schema_path" in gates. An explicitnullvalue satisfies that test, andrepo_root / Nonethen raisesTypeErrorinstead of falling back toFROZEN_TOOLS. This recipe sets a real string, so nothing breaks now. The null case is realistic here, because this file's own comment records that the v2 configs previously set a schema gate field to null on purpose.The path selection is otherwise correct and backward compatible, and it matches foundry_promote.py lines 445-447.
♻️ Proposed change
expected_hash = gates.get("tool_schema_hash") if expected_hash: - frozen_tools_path = (repo_root / gates["tool_schema_path"]) if "tool_schema_path" in gates else FROZEN_TOOLS + schema_path_rel = gates.get("tool_schema_path") + frozen_tools_path = (repo_root / schema_path_rel) if schema_path_rel else FROZEN_TOOLS🤖 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 `@training_pit/foundry/scripts/foundry_preflight.py` around lines 137 - 142, Update the path selection in the tool schema validation block using gates.get("tool_schema_path") so an explicit null or missing value falls back to FROZEN_TOOLS. Preserve the existing repo_root path resolution for non-empty configured paths and the surrounding expected_hash validation behavior.training_pit/foundry/configs/toolcaller_v2_r3.json (1)
106-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSet
arena_json_validity_minso gate#9enforces a real floor.
promotion.margin.rulesdoes not definearena_json_validity_min. foundry_promote.py line 381 reads it withrules.get("arena_json_validity_min", 0), so the floor defaults to 0 and the check cannot fail. The registry entry records the vacuous result:"candidate 0.9808 >= floor 0"(PROMOTION_REGISTRY.json line 93).This round adds two other explicit floors to
rules. Add this one too, soarena_json_validity_mingates JSON validity instead of only reporting it.♻️ Proposed floor
"arena_decision_accuracy_max_drop": 0.02, + "arena_json_validity_min": 0.95, + "_arena_json_validity_min_note": "Previously unset, so foundry_promote.py's rules.get(..., 0) fallback made gate `#9` pass against a floor of 0. Pinned below r3's measured 0.9808 so a real JSON-validity regression fails the gate.", "generalization_accuracy_min": 0.85,🤖 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 `@training_pit/foundry/configs/toolcaller_v2_r3.json` around lines 106 - 114, Add an explicit nonzero arena_json_validity_min threshold to the promotion.margin.rules object alongside the other promotion floors, using the intended proposed floor for this round. Ensure the existing gate in foundry_promote.py reads this configured value rather than defaulting to 0, without changing the surrounding rules.training_pit/foundry/scripts/foundry_promote.py (2)
268-270: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard
meta['sources']on the no-leakage path.Line 270 subscripts
meta['sources']directly. This branch is selected only by"isolation" not in meta, and the comment at lines 250-253 states thatisolationabsence is the discriminator. Nothing guarantees the same meta also definessources.The consequence is inverted: the subscript sits on the
c.okpath, so a composed meta withoutsourcescrashes the script exactly when train and eval are disjoint, while a leaking dataset still reports its failure correctly. This round's sidecar defines 3 sources (toolcaller_v2_r3.meta.json lines 6-25), so the gate passed (PROMOTION_REGISTRY.json line 53).♻️ Proposed change
else: + stream_count = len(meta.get("sources") or {}) c.ok(f"train ({len(train_ids)}) and eval ({len(eval_ids)}) example_ids are " - f"fully disjoint across {len(meta['sources'])} composed stream(s), no leakage") + f"fully disjoint across {stream_count} composed stream(s), no leakage")🤖 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 `@training_pit/foundry/scripts/foundry_promote.py` around lines 268 - 270, Guard the sources count in the no-leakage branch of the promotion flow so missing meta['sources'] cannot crash the successful c.ok path. Update the message near the train/eval disjointness check to use a safe fallback when sources is absent, while preserving the existing count when meta['sources'] is defined.
126-131: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueReport a missing
example_idas a gate failure, not a traceback.Line 130 indexes
["example_id"]directly. A row without that key raisesKeyErrorand aborts foundry_promote.py with an unhandled traceback, before any check result prints. The composed dataset mixes two capture schemas ("schema_version": "mixed (toolcaller-v0, toolcaller-v2)", toolcaller_v2_r3.meta.json line 3), so row shape is not uniform by construction.Every current row carries the key, because the gate passed with 2572 and 428 ids (PROMOTION_REGISTRY.json line 53). Convert the crash into a readable
frozen_group_splitfailure so a future stream with a different row shape fails the gate instead of the script.♻️ Proposed change
-def load_example_ids(path: Path) -> set[str]: +def load_example_ids(path: Path) -> tuple[set[str], int]: """example_id set of a chat-format JSONL dataset file, for the toolcaller-v2 composed- multi-stream branch of the frozen_group_split gate below -- a direct, on-disk leakage check (do train and eval actually share any example_id?) rather than v0's parameter-match check (did the generator run with the frozen frac/seed?), since v2 datasets aren't built by one frac/seed split, they're concatenated from independently-generated, independently - lineage-split streams.""" + lineage-split streams. Also returns the count of rows with NO example_id: such a row cannot + be leakage-checked at all, so the gate must fail loudly instead of silently ignoring it.""" ids: set[str] = set() + missing = 0 with path.open(encoding="utf-8") as fh: for line in fh: line = line.strip() if line: - ids.add(json.loads(line)["example_id"]) - return ids + example_id = json.loads(line).get("example_id") + if example_id is None: + missing += 1 + else: + ids.add(example_id) + return ids, missingThen update the two call sites:
train_ids, train_missing = load_example_ids(train_path) eval_ids, eval_missing = load_example_ids(eval_path_for_split) overlap = train_ids & eval_ids if overlap: c.fail(f"{len(overlap)} example_id(s) appear in BOTH train and eval " f"(e.g. {next(iter(overlap))}) -- real train/eval leakage") elif train_missing or eval_missing: c.fail(f"{train_missing} train / {eval_missing} eval row(s) carry no " "example_id -- cannot verify those rows are not leaked") else: c.ok(f"train ({len(train_ids)}) and eval ({len(eval_ids)}) example_ids are " f"fully disjoint across {len(meta['sources'])} composed stream(s), no leakage")🤖 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 `@training_pit/foundry/scripts/foundry_promote.py` around lines 126 - 131, Update load_example_ids to tolerate rows missing example_id by returning both the collected ID set and a count of missing IDs instead of raising KeyError. Update both frozen_group_split call sites to unpack those results, fail with the specified readable message when either count is nonzero, and retain overlap checking before reporting success.training_pit/foundry/scripts/eval_toolcaller.py (1)
216-219: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClose the dump file through a context manager so a crash keeps the records.
failures_fhopens at line 219 and closes at line 321 with notry/finallyand no context manager. The evaluation loop can raise outside the guarded generate block.compute_metricsandwrite_progressat lines 315-316, and thejson.dumpscalls at lines 274 and 305, are all unguarded. On such an exception line 321 never runs, so buffered records are lost and the dump is truncated or empty. That removes the diagnostic exactly when a crash makes it most useful.Use
contextlib.ExitStackto keep the optional-file shape and still guarantee the flush.♻️ Proposed change
Add the import near the top of the file:
import contextlibThen wrap the evaluation loop:
results: list[dict] = [] - failures_fh = None - if args.dump_failures is not None: - args.dump_failures.parent.mkdir(parents=True, exist_ok=True) - failures_fh = args.dump_failures.open("w", encoding="utf-8") - - for i, row in enumerate(eval_rows): + with contextlib.ExitStack() as stack: + failures_fh = None + if args.dump_failures is not None: + args.dump_failures.parent.mkdir(parents=True, exist_ok=True) + failures_fh = stack.enter_context(args.dump_failures.open("w", encoding="utf-8")) + + for i, row in enumerate(eval_rows): + ... # loop body unchanged, indented one levelThen replace the manual close with the report only:
- if failures_fh is not None: - failures_fh.close() - print(f"Wrote decision-mismatch dump to {args.dump_failures}", flush=True) + if args.dump_failures is not None: + print(f"Wrote decision-mismatch dump to {args.dump_failures}", flush=True)Also applies to: 320-322
🤖 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 `@training_pit/foundry/scripts/eval_toolcaller.py` around lines 216 - 219, Use contextlib.ExitStack around the evaluation flow in the main execution path, entering the optional args.dump_failures file handle when configured so it is flushed and closed even if the loop, compute_metrics, write_progress, or json.dumps raises. Replace the manual failures_fh.close() cleanup with reporting after the ExitStack-managed block, while preserving the existing optional-file behavior.training_pit/foundry/scripts/generate_toolcaller_v2_dataset.py (1)
224-240: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winOversample
explicit_call_scenariosso rejected captures do not fail the top-up run.
explicit_call_scenariosreturns exactlyargs.countcandidate scenarios. The generation loop discards scenarios on backend errors, missing JSON, or validation failure, and then exits 1 whengenerated < args.count. Keep the headroom fromplan_scenarios_v2, such ascount * 4followed bybreakatcount * 3.🤖 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 `@training_pit/foundry/scripts/generate_toolcaller_v2_dataset.py` around lines 224 - 240, Update explicit_call_scenarios to generate headroom beyond the requested count, such as iterating up to count * 4 while retaining only count * 3 candidates. Preserve the existing scenario construction and return structure so rejected captures can be replaced during the top-up run.
🤖 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.
Outside diff comments:
In `@training_pit/foundry/scripts/foundry_preflight.py`:
- Around line 145-147: Update the tool_schema_hash comparison in the preflight
metadata validation so it runs only when the sidecar records a non-null hash.
Preserve the existing mismatch finding when a hash is present, and otherwise
rely on the frozen file hash check already performed by the preflight flow,
matching the behavior in foundry_promote.py.
---
Nitpick comments:
In `@training_pit/foundry/configs/toolcaller_v2_r3.json`:
- Around line 106-114: Add an explicit nonzero arena_json_validity_min threshold
to the promotion.margin.rules object alongside the other promotion floors, using
the intended proposed floor for this round. Ensure the existing gate in
foundry_promote.py reads this configured value rather than defaulting to 0,
without changing the surrounding rules.
In `@training_pit/foundry/scripts/eval_toolcaller.py`:
- Around line 216-219: Use contextlib.ExitStack around the evaluation flow in
the main execution path, entering the optional args.dump_failures file handle
when configured so it is flushed and closed even if the loop, compute_metrics,
write_progress, or json.dumps raises. Replace the manual failures_fh.close()
cleanup with reporting after the ExitStack-managed block, while preserving the
existing optional-file behavior.
In `@training_pit/foundry/scripts/foundry_preflight.py`:
- Around line 137-142: Update the path selection in the tool schema validation
block using gates.get("tool_schema_path") so an explicit null or missing value
falls back to FROZEN_TOOLS. Preserve the existing repo_root path resolution for
non-empty configured paths and the surrounding expected_hash validation
behavior.
In `@training_pit/foundry/scripts/foundry_promote.py`:
- Around line 268-270: Guard the sources count in the no-leakage branch of the
promotion flow so missing meta['sources'] cannot crash the successful c.ok path.
Update the message near the train/eval disjointness check to use a safe fallback
when sources is absent, while preserving the existing count when meta['sources']
is defined.
- Around line 126-131: Update load_example_ids to tolerate rows missing
example_id by returning both the collected ID set and a count of missing IDs
instead of raising KeyError. Update both frozen_group_split call sites to unpack
those results, fail with the specified readable message when either count is
nonzero, and retain overlap checking before reporting success.
In `@training_pit/foundry/scripts/generate_toolcaller_v2_dataset.py`:
- Around line 224-240: Update explicit_call_scenarios to generate headroom
beyond the requested count, such as iterating up to count * 4 while retaining
only count * 3 candidates. Preserve the existing scenario construction and
return structure so rejected captures can be replaced during the top-up run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 17fa59f5-63e4-4afe-894f-476d42cfdeec
📒 Files selected for processing (8)
training_pit/datasets/toolcaller_v2_explicit_call_r3.meta.jsontraining_pit/datasets/toolcaller_v2_r3.meta.jsontraining_pit/foundry/PROMOTION_REGISTRY.jsontraining_pit/foundry/configs/toolcaller_v2_r3.jsontraining_pit/foundry/scripts/eval_toolcaller.pytraining_pit/foundry/scripts/foundry_preflight.pytraining_pit/foundry/scripts/foundry_promote.pytraining_pit/foundry/scripts/generate_toolcaller_v2_dataset.py
1 participant
Summary
Round 2 fixed round 1's
plausible_fabricationsafety regression (63.9% → 95.5% cp95-lower) but traded away generalization accuracy in the process (93.3% → 81.3% on the held-out generalization arena, missing the 85% floor). Round 3 fixes that without giving back the safety gain.Diagnosis (new
eval_toolcaller.py --dump-failuresflag): 24/28 mismatches were the model predicting{"decision": "unsupported", "reason_code": "no_matching_tool"}for a REAL held-out-family tool whose full schema was present inavailable_tools— the exact response shape round 2's fabrication-decoy training taught, misfiring on a target it should have called. Root cause: every round-2 training row using explicit-name-drop phrasing ("use X to...", "call the X tool so we can...") was a decoy (correct answer: refuse); normal synthetic-tool call scenarios never used that phrasing, so the model learned the phrasing pattern itself as weak evidence for refusal.Fix:
explicit_call_scenarios(generate_toolcaller_v2_dataset.py --explicit-call-only) — the direct counterbalance: same explicit-name-drop phrasing as the decoys, but targeting a real/train-pool-synthetic tool that is inavailable_tools, expected decision "call". 117 fresh captures (~1:1 against round 2's 120 decoys), composed into round 3's training set alongside the unchanged v0 and v2-bulk-r2 streams.Round 3 results (all three real eval sets):
Promotion gate fixes — round 2's config left 3 gates as "deliberate config-posture gaps." Digging in during round 3 found two were real, fixable defects and the third needed a genuinely different check for v2's dataset shape:
sealed_eval_hash: just needed the config to record the eval file's real current hash.runtime_schema_identity:foundry_preflight.pyhad its own hardcodedtool_schema_hashcheck with notool_schema_pathoverride support (separate fromfoundry_promote.py's gate, which already supported it — why r2 left this null). Fixed at the root; verified byte-identical preflight output on the v0 config before/after.frozen_group_split: v0's gate checks "did the generator run with the frozen frac/seed," which doesn't apply to v2's concatenated-independently-lineage-split-streams shape. Added a v2-specific branch that directly verifies zeroexample_idoverlap between the composed train/eval files on disk — a real leakage check, not a parameter-trust one.Round 3 promoted: all 16 gates pass, approved by hardcoreerik, registry entry written. Candidate GGUF exported but not yet deployed (separate, later manual step).
Test plan
explicit_call_scenariossmoke-tested (5 rows) before the real 150-attempt batchToolcallerBenchPASS 117/117 on the new stream at exportfoundry_preflight.pyfix verified byte-identical ontoolcaller_v0_r3.jsonbefore/after (no regression)frozen_group_split's new branch verified against real train/eval files (2572/428 example_ids, zero overlap)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation