Training Pit: toolcaller synthetic generator + Stage 1 target switcher - #43
Merged
Merged
Conversation
#43) - New generate_toolcaller_dataset.py: seeded scenario generator that produces balanced toolcaller-v0 capture JSON files (40% call / 30% no_tool / 20% clarify / 10% unsupported across all 4 roles). Decision type is predetermined by recipe — Ollama model only fills in realistic request text and arguments, preventing label drift. Outputs to training_pit/datasets/ toolcaller/ in the capture schema format ready for ToolcallerBench + export. - Stage 1 Generate Dataset now has a "Generation Target" picker: Boss Plans -> ORC ACADEMY (Stage 2) [existing behavior] Toolcaller Examples -> THE FOUNDRY (Stage 3) [new] Switching target auto-updates the key field, description, and hint text. After a successful toolcaller run the "Captures ready" button appears and triggers RefreshFoundry() on click. - Stage 3 THE FOUNDRY gate card shows live progress toward the 150-capture training gate with color-coded bars and an ETA estimate. - Stage 1 route labels: "-> Stage 2" / "swarm captures" badges added to column headers so the two distinct data paths are visually unambiguous. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR adds generation target routing and Foundry activation in the Training Pit, introduces a toolcaller dataset generator, and expands conversion tooling for Qwen models and LoRA adapters. ChangesTraining Pit generation flow and Foundry activation
Toolcaller dataset generator script
Model conversion and LoRA export tooling
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml (1)
438-467: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winModel/Key tooltips remain boss-plan specific after switching to the Toolcaller target.
ToolTip.Tip="Local Ollama model that will generate boss plans..."(Model) and"Output dataset key — saves train_{key}.jsonl and eval_{key}.jsonl"(Key) stay static regardless ofCbGenTargetselection. Per the PR summary, toolcaller output is a directory of per-capture JSON files undertraining_pit/datasets/toolcaller/, nottrain_{key}.jsonl/eval_{key}.jsonl— so this tooltip becomes misleading once Toolcaller is selected, and the "📄 File" button (which checks fortrain_{key}.jsonl) will simply stay disabled for that path with no explanation.🤖 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 `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml` around lines 438 - 467, Update the Model and Key tooltip text in TrainingPitPanel so it reflects the currently selected generation target from CbGenTarget instead of always describing boss-plan JSONL output. In the UI bindings for the Model/Key fields, make the ToolTip.Tip content conditional on the target: keep the boss-plan wording for boss-plan mode, but describe the Toolcaller dataset directory and per-capture JSON file output when Toolcaller is selected. Also ensure any tooltip or hint around the file-related action matches the same target-specific path semantics.
🧹 Nitpick comments (1)
OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs (1)
1585-1636: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExport-gate detection relies on magic-string sentinels set in a different method.
exportOkis derived fromexported != "not exported" && exported != "meta unreadable", but those exact strings are only defined inRefreshFoundry(lines 1555, 1566). Any future tweak to that wording in one place and not the other silently breaks the export gate's icon/color/label without a compile error.♻️ Proposed refactor — pass a bool instead of sentinel strings
- UpdateFoundryGateCard(staged, accepted, exported); + UpdateFoundryGateCard(staged, accepted, exported, exportOk: File.Exists(metaPath) && exported != "meta unreadable");And change
UpdateFoundryGateCardto acceptbool exportOkdirectly instead of re-deriving it from string comparison.🤖 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 `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs` around lines 1585 - 1636, The export gate logic in UpdateFoundryGateCard is re-deriving state from magic strings, which should be replaced with a boolean flow. Update RefreshFoundry to compute a real export success flag and pass it into UpdateFoundryGateCard instead of the exported status string, then change UpdateFoundryGateCard to take that bool and drive GateExportIcon, GateExportLabel, and related colors directly from it. This keeps the export-gate UI in sync with the source of truth and avoids brittle string comparisons.
🤖 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 `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml`:
- Around line 407-410: The “→ Stage 2” badge is hardcoded and cannot be updated
when the target selection changes, so it stays inconsistent with the selected
route. Add an x:Name to the TextBlock in TrainingPitPanel.axaml and update its
Text in CbGenTarget_SelectionChanged alongside TbGenTargetNote.Text, using the
same selection logic that distinguishes the Stage 2 vs Stage 3 path.
In `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs`:
- Around line 1211-1234: The dataset lookup in BtnGenLoadAcademy_Click should
use the locked-in _genKey from BtnGenStart_Click instead of preferring the
mutable TbGenKey textbox, since the textbox may have been edited after
generation finished. Update the key selection logic so the generated dataset is
located by the original captured key first (falling back only if appropriate),
and keep the existing dataset matching flow against CbDataset.ItemsSource,
trainPath, and DatasetOptionAva unchanged otherwise.
---
Outside diff comments:
In `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml`:
- Around line 438-467: Update the Model and Key tooltip text in TrainingPitPanel
so it reflects the currently selected generation target from CbGenTarget instead
of always describing boss-plan JSONL output. In the UI bindings for the
Model/Key fields, make the ToolTip.Tip content conditional on the target: keep
the boss-plan wording for boss-plan mode, but describe the Toolcaller dataset
directory and per-capture JSON file output when Toolcaller is selected. Also
ensure any tooltip or hint around the file-related action matches the same
target-specific path semantics.
---
Nitpick comments:
In `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs`:
- Around line 1585-1636: The export gate logic in UpdateFoundryGateCard is
re-deriving state from magic strings, which should be replaced with a boolean
flow. Update RefreshFoundry to compute a real export success flag and pass it
into UpdateFoundryGateCard instead of the exported status string, then change
UpdateFoundryGateCard to take that bool and drive GateExportIcon,
GateExportLabel, and related colors directly from it. This keeps the export-gate
UI in sync with the source of truth and avoids brittle string comparisons.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 012ec036-e42c-466b-b9d4-736a3b621620
📒 Files selected for processing (3)
OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axamlOrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cstraining_pit/foundry/scripts/generate_toolcaller_dataset.py
Comment on lines
+407
to
+410
| <Border Background="#0A1A10" BorderBrush="#2A5A38" BorderThickness="1" | ||
| CornerRadius="3" Padding="5,1" Margin="8,0,0,0" VerticalAlignment="Center"> | ||
| <TextBlock Text="→ Stage 2" FontSize="9" Foreground="#60A070"/> | ||
| </Border> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Static "→ Stage 2" badge doesn't reflect the new target picker.
This badge has no x:Name, so it can never be updated when CbGenTarget_SelectionChanged fires. Once the user selects "Toolcaller Examples → THE FOUNDRY", the output actually feeds Stage 3, but this badge still reads "→ Stage 2" — directly contradicting the PR's own goal of "route labels ... to distinguish the two data paths." The TbGenTargetNote text below is correctly updated, but this badge sits right in the header and will visibly conflict with it.
🏷️ Proposed fix
- <Border Background="`#0A1A10`" BorderBrush="`#2A5A38`" BorderThickness="1"
+ <Border x:Name="GenRouteBadge" Background="`#0A1A10`" BorderBrush="`#2A5A38`" BorderThickness="1"
CornerRadius="3" Padding="5,1" Margin="8,0,0,0" VerticalAlignment="Center">
- <TextBlock Text="→ Stage 2" FontSize="9" Foreground="`#60A070`"/>
+ <TextBlock x:Name="GenRouteBadgeText" Text="→ Stage 2" FontSize="9" Foreground="`#60A070`"/>
</Border>Then update GenRouteBadgeText.Text alongside TbGenTargetNote.Text inside CbGenTarget_SelectionChanged.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Border Background="#0A1A10" BorderBrush="#2A5A38" BorderThickness="1" | |
| CornerRadius="3" Padding="5,1" Margin="8,0,0,0" VerticalAlignment="Center"> | |
| <TextBlock Text="→ Stage 2" FontSize="9" Foreground="#60A070"/> | |
| </Border> | |
| <Border x:Name="GenRouteBadge" Background="`#0A1A10`" BorderBrush="`#2A5A38`" BorderThickness="1" | |
| CornerRadius="3" Padding="5,1" Margin="8,0,0,0" VerticalAlignment="Center"> | |
| <TextBlock x:Name="GenRouteBadgeText" Text="→ Stage 2" FontSize="9" Foreground="`#60A070`"/> | |
| </Border> |
🤖 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 `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml` around lines 407 -
410, The “→ Stage 2” badge is hardcoded and cannot be updated when the target
selection changes, so it stays inconsistent with the selected route. Add an
x:Name to the TextBlock in TrainingPitPanel.axaml and update its Text in
CbGenTarget_SelectionChanged alongside TbGenTargetNote.Text, using the same
selection logic that distinguishes the Stage 2 vs Stage 3 path.
Comment on lines
+1211
to
+1234
| private void BtnGenLoadAcademy_Click(object? s, RoutedEventArgs e) | ||
| { | ||
| var key = TbGenKey.Text?.Trim() ?? _genKey; | ||
| if (key.Length == 0) return; | ||
| var dsDir = Path.Combine(_pitRoot, "training_pit", "datasets"); | ||
| var trainPath = Path.Combine(dsDir, $"train_{key}.jsonl"); | ||
| var match = (CbDataset.ItemsSource as IEnumerable<DatasetOptionAva>) | ||
| ?.FirstOrDefault(d => d.TrainPath == trainPath || d.Name == key); | ||
| if (match is not null) | ||
| { | ||
| CbDataset.SelectedItem = match; | ||
| OnActivity?.Invoke($"→ Dataset '{key}' loaded into ORC ACADEMY — configure the base model in Stage 2 and click Start training."); | ||
| } | ||
| else | ||
| { | ||
| _ = ReloadModelsAsync().ContinueWith(_ => Dispatcher.UIThread.Post(() => | ||
| { | ||
| var m2 = (CbDataset.ItemsSource as IEnumerable<DatasetOptionAva>) | ||
| ?.FirstOrDefault(d => d.TrainPath == trainPath || d.Name == key); | ||
| if (m2 is not null) { CbDataset.SelectedItem = m2; OnActivity?.Invoke($"→ Dataset '{key}' loaded into ORC ACADEMY."); } | ||
| else OnActivity?.Invoke($"→ Dataset '{key}' not found — check training_pit/datasets/train_{key}.jsonl exists."); | ||
| }), TaskScheduler.Default); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prefer the locked-in _genKey over the live textbox for locating the just-generated dataset.
var key = TbGenKey.Text?.Trim() ?? _genKey; lets the mutable TbGenKey textbox take precedence over _genKey, which was captured at the moment generation started (BtnGenStart_Click line 990). If the user edits the key field after a run completes but before clicking "Use this dataset in ORC ACADEMY" (e.g., prepping the key for a next run), this will try to load a dataset under the new key instead of the one that was actually just generated — silently loading an unrelated/stale dataset if one happens to exist under that name.
🔑 Proposed fix — prefer the locked-in key
- var key = TbGenKey.Text?.Trim() ?? _genKey;
+ var key = _genKey.Length > 0 ? _genKey : (TbGenKey.Text?.Trim() ?? "");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private void BtnGenLoadAcademy_Click(object? s, RoutedEventArgs e) | |
| { | |
| var key = TbGenKey.Text?.Trim() ?? _genKey; | |
| if (key.Length == 0) return; | |
| var dsDir = Path.Combine(_pitRoot, "training_pit", "datasets"); | |
| var trainPath = Path.Combine(dsDir, $"train_{key}.jsonl"); | |
| var match = (CbDataset.ItemsSource as IEnumerable<DatasetOptionAva>) | |
| ?.FirstOrDefault(d => d.TrainPath == trainPath || d.Name == key); | |
| if (match is not null) | |
| { | |
| CbDataset.SelectedItem = match; | |
| OnActivity?.Invoke($"→ Dataset '{key}' loaded into ORC ACADEMY — configure the base model in Stage 2 and click Start training."); | |
| } | |
| else | |
| { | |
| _ = ReloadModelsAsync().ContinueWith(_ => Dispatcher.UIThread.Post(() => | |
| { | |
| var m2 = (CbDataset.ItemsSource as IEnumerable<DatasetOptionAva>) | |
| ?.FirstOrDefault(d => d.TrainPath == trainPath || d.Name == key); | |
| if (m2 is not null) { CbDataset.SelectedItem = m2; OnActivity?.Invoke($"→ Dataset '{key}' loaded into ORC ACADEMY."); } | |
| else OnActivity?.Invoke($"→ Dataset '{key}' not found — check training_pit/datasets/train_{key}.jsonl exists."); | |
| }), TaskScheduler.Default); | |
| } | |
| } | |
| private void BtnGenLoadAcademy_Click(object? s, RoutedEventArgs e) | |
| { | |
| var key = _genKey.Length > 0 ? _genKey : (TbGenKey.Text?.Trim() ?? ""); | |
| if (key.Length == 0) return; | |
| var dsDir = Path.Combine(_pitRoot, "training_pit", "datasets"); | |
| var trainPath = Path.Combine(dsDir, $"train_{key}.jsonl"); | |
| var match = (CbDataset.ItemsSource as IEnumerable<DatasetOptionAva>) | |
| ?.FirstOrDefault(d => d.TrainPath == trainPath || d.Name == key); | |
| if (match is not null) | |
| { | |
| CbDataset.SelectedItem = match; | |
| OnActivity?.Invoke($"→ Dataset '{key}' loaded into ORC ACADEMY — configure the base model in Stage 2 and click Start training."); | |
| } | |
| else | |
| { | |
| _ = ReloadModelsAsync().ContinueWith(_ => Dispatcher.UIThread.Post(() => | |
| { | |
| var m2 = (CbDataset.ItemsSource as IEnumerable<DatasetOptionAva>) | |
| ?.FirstOrDefault(d => d.TrainPath == trainPath || d.Name == key); | |
| if (m2 is not null) { CbDataset.SelectedItem = m2; OnActivity?.Invoke($"→ Dataset '{key}' loaded into ORC ACADEMY."); } | |
| else OnActivity?.Invoke($"→ Dataset '{key}' not found — check training_pit/datasets/train_{key}.jsonl exists."); | |
| }), TaskScheduler.Default); | |
| } | |
| } |
🤖 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 `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs` around lines
1211 - 1234, The dataset lookup in BtnGenLoadAcademy_Click should use the
locked-in _genKey from BtnGenStart_Click instead of preferring the mutable
TbGenKey textbox, since the textbox may have been edited after generation
finished. Update the key selection logic so the generated dataset is located by
the original captured key first (falling back only if appropriate), and keep the
existing dataset matching flow against CbDataset.ItemsSource, trainPath, and
DatasetOptionAva unchanged otherwise.
generate_toolcaller_dataset.py: add --api claude|ollama flag (auto-detects ANTHROPIC_API_KEY), claude_generate() function via Anthropic REST API (requests-only, no SDK), --claude-model arg (default haiku-4-5-20251001), and teacher_model provenance field set correctly for Claude-generated captures. Stage 1 UI: GENERATION BACKEND picker card (visible in toolcaller mode only) lets user choose Claude API vs Ollama local. Model picker is still shown but skipped when Claude is selected. Activity log reports which backend is active. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
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)
training_pit/foundry/scripts/generate_toolcaller_dataset.py (1)
358-360: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequired-argument check drops legitimate falsy values.
if vtreats0,False, and""as missing, so a validcallwhose required parameter is legitimately0orFalsegets rejected. Check for key presence and non-null/non-empty instead of truthiness.🐛 Proposed fix
- # All required params present and non-empty - if not required_params.issubset(set(k for k, v in arguments.items() if v)): - return None + # All required params present and non-empty + present = {k for k, v in arguments.items() if v not in (None, "")} + if not required_params.issubset(present): + return None🤖 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_dataset.py` around lines 358 - 360, The required-argument validation in generate_toolcaller_dataset.py is incorrectly using truthiness, so legitimate falsy values like 0 or False get treated as missing. Update the check in the block that builds the argument set to validate presence and only exclude truly absent or empty values, preserving valid required params passed through call/arguments handling.
🧹 Nitpick comments (1)
training_pit/foundry/scripts/generate_toolcaller_dataset.py (1)
584-598: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo retry/backoff on transient API failures (esp. HTTP 429).
Any exception—including Anthropic rate-limit
429s—counts the scenario as rejected and permanently discards that slot. Under sustained throttling this can silently deplete the over-generated scenario buffer and cause a shortfall exit. Anthropic returnsretry-afteron 429s; consider a bounded retry with exponential backoff (honoringretry-after) before treating the scenario as failed.claude_generate/ollama_generateraise on non-2xx viaraise_for_status(), so arequests.HTTPErroris distinguishable from parse failures here.🤖 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_dataset.py` around lines 584 - 598, The current exception handler in generate_toolcaller_dataset.py immediately rejects scenarios on any failure, which drops transient API throttling cases like HTTP 429. Update the retry path around the claude_generate/ollama_generate call to perform a bounded retry with exponential backoff, and when the error is a requests.HTTPError from claude_generate honor any retry-after value before retrying. Only increment rejected and continue after retries are exhausted, and keep the existing backend-specific logging in the try/except block.
🤖 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 `@training_pit/foundry/scripts/generate_toolcaller_dataset.py`:
- Line 540: The toolcaller dataset generator startup print uses an unnecessary
f-string with no placeholders, triggering Ruff F541. Update the print statement
in generate_toolcaller_dataset.py to use a plain string literal instead of an
f-string, keeping the same message and flush behavior. Locate the change in the
startup logging near the generate_toolcaller_dataset script entrypoint.
---
Outside diff comments:
In `@training_pit/foundry/scripts/generate_toolcaller_dataset.py`:
- Around line 358-360: The required-argument validation in
generate_toolcaller_dataset.py is incorrectly using truthiness, so legitimate
falsy values like 0 or False get treated as missing. Update the check in the
block that builds the argument set to validate presence and only exclude truly
absent or empty values, preserving valid required params passed through
call/arguments handling.
---
Nitpick comments:
In `@training_pit/foundry/scripts/generate_toolcaller_dataset.py`:
- Around line 584-598: The current exception handler in
generate_toolcaller_dataset.py immediately rejects scenarios on any failure,
which drops transient API throttling cases like HTTP 429. Update the retry path
around the claude_generate/ollama_generate call to perform a bounded retry with
exponential backoff, and when the error is a requests.HTTPError from
claude_generate honor any retry-after value before retrying. Only increment
rejected and continue after retries are exhausted, and keep the existing
backend-specific logging in the try/except block.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 97c1834e-b8d1-48f5-a375-78f31ae496a5
📒 Files selected for processing (3)
OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axamlOrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cstraining_pit/foundry/scripts/generate_toolcaller_dataset.py
🚧 Files skipped from review as they are similar to previous changes (2)
- OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml
- OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs
| active_model = args.model | ||
| teacher_model = None # Ollama model is the producer, no distillation teacher | ||
|
|
||
| print(f"=== toolcaller dataset generator v0 ===", flush=True) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove extraneous f prefix (Ruff F541).
This f-string has no placeholders; Ruff reports it as an error, which can fail lint-gated CI.
🧹 Proposed fix
- print(f"=== toolcaller dataset generator v0 ===", flush=True)
+ print("=== toolcaller dataset generator v0 ===", flush=True)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| print(f"=== toolcaller dataset generator v0 ===", flush=True) | |
| print("=== toolcaller dataset generator v0 ===", flush=True) |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 540-540: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 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_dataset.py` at line 540, The
toolcaller dataset generator startup print uses an unnecessary f-string with no
placeholders, triggering Ruff F541. Update the print statement in
generate_toolcaller_dataset.py to use a plain string literal instead of an
f-string, keeping the same message and flush behavior. Locate the change in the
startup logging near the generate_toolcaller_dataset script entrypoint.
Source: Linters/SAST tools
generate_toolcaller_dataset.py: --api native calls the llama.cpp server at http://127.0.0.1:8080 via POST /v1/chat/completions (OpenAI-compat, non-streaming). native_generate() + native_list_models() added. Model auto-discovered from /v1/models when not specified. Health probe uses GET /health. Auto-detect priority: native (if /health 200) > claude (if ANTHROPIC_API_KEY) > ollama. Stage 1 UI: GENERATION BACKEND picker now has three options — Native Runtime (default, fully local), Claude API, Ollama. _genBackend is now a string enum ("native"|"claude"|"ollama") replacing the bool. Note text updates per selection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
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 `@training_pit/foundry/scripts/generate_toolcaller_dataset.py`:
- Around line 572-589: The native model provenance handling in the model
selection block should not persist the fallback sentinel as teacher metadata. In
the args.api == "native" path, keep using "native-unknown" for active_model when
discovery fails, but set teacher_model to None whenever the resolved model name
is unknown instead of copying active_model. Update the logic around
native_list_models, active_model, and teacher_model so only a real discovered or
user-specified native model is recorded as provenance.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 47b6b4f1-cfbf-49a7-af07-264a546656ca
📒 Files selected for processing (3)
OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axamlOrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cstraining_pit/foundry/scripts/generate_toolcaller_dataset.py
🚧 Files skipped from review as they are similar to previous changes (1)
- OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml
Comment on lines
+572
to
+589
| # Determine canonical model name and teacher provenance | ||
| if args.api == "native": | ||
| # Discover model from runtime if not explicitly set | ||
| if args.model and args.model != "qwen2.5-coder:14b": | ||
| active_model = args.model # user specified a native model name | ||
| else: | ||
| try: | ||
| found = native_list_models(args.native_host) | ||
| active_model = found[0] if found else "native-unknown" | ||
| except Exception: | ||
| active_model = "native-unknown" | ||
| teacher_model: str | None = active_model # local model acts as teacher | ||
| elif args.api == "claude": | ||
| active_model = args.claude_model | ||
| teacher_model = args.claude_model # Claude acts as teacher for Qwen student | ||
| else: | ||
| active_model = args.model | ||
| teacher_model = None # Ollama is producer, no separate distillation teacher |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Avoid recording the native-unknown placeholder as teacher_model provenance.
When model discovery fails (empty /v1/models or exception), active_model becomes the sentinel "native-unknown", which then flows into teacher_model and gets persisted into every capture's provenance. That pollutes the dataset lineage with a fake teacher identity. Prefer leaving teacher_model as None when the real model name is unknown.
🛠️ Proposed fix
try:
found = native_list_models(args.native_host)
active_model = found[0] if found else "native-unknown"
except Exception:
active_model = "native-unknown"
- teacher_model: str | None = active_model # local model acts as teacher
+ # Only record a teacher identity when the real model name is known.
+ teacher_model: str | None = active_model if active_model != "native-unknown" else None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Determine canonical model name and teacher provenance | |
| if args.api == "native": | |
| # Discover model from runtime if not explicitly set | |
| if args.model and args.model != "qwen2.5-coder:14b": | |
| active_model = args.model # user specified a native model name | |
| else: | |
| try: | |
| found = native_list_models(args.native_host) | |
| active_model = found[0] if found else "native-unknown" | |
| except Exception: | |
| active_model = "native-unknown" | |
| teacher_model: str | None = active_model # local model acts as teacher | |
| elif args.api == "claude": | |
| active_model = args.claude_model | |
| teacher_model = args.claude_model # Claude acts as teacher for Qwen student | |
| else: | |
| active_model = args.model | |
| teacher_model = None # Ollama is producer, no separate distillation teacher | |
| # Determine canonical model name and teacher provenance | |
| if args.api == "native": | |
| # Discover model from runtime if not explicitly set | |
| if args.model and args.model != "qwen2.5-coder:14b": | |
| active_model = args.model # user specified a native model name | |
| else: | |
| try: | |
| found = native_list_models(args.native_host) | |
| active_model = found[0] if found else "native-unknown" | |
| except Exception: | |
| active_model = "native-unknown" | |
| # Only record a teacher identity when the real model name is known. | |
| teacher_model: str | None = active_model if active_model != "native-unknown" else None | |
| elif args.api == "claude": | |
| active_model = args.claude_model | |
| teacher_model = args.claude_model # Claude acts as teacher for Qwen student | |
| else: | |
| active_model = args.model | |
| teacher_model = None # Ollama is producer, no separate distillation teacher |
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 581-581: Do not catch blind exception: Exception
(BLE001)
🤖 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_dataset.py` around lines 572
- 589, The native model provenance handling in the model selection block should
not persist the fallback sentinel as teacher metadata. In the args.api ==
"native" path, keep using "native-unknown" for active_model when discovery
fails, but set teacher_model to None whenever the resolved model name is unknown
instead of copying active_model. Update the logic around native_list_models,
active_model, and teacher_model so only a real discovered or user-specified
native model is recorded as provenance.
…#43-followup) - convert_lora_to_gguf.py + conversion/ package from llama.cpp for PEFT→GGUF conversion - GGUF LoRA adapter (70.5 MB) + Qwen2.5-1.5B-Instruct-Q4_K_M.gguf (940 MB) placed in depot - LlamaServerManager: LoraPath property + --lora flag in BuildArgs() - AppSettings: LlamaCppLoraPath persisted setting - MainWindow: wires LoraPath when building server manager; ApplyFoundryAdapter() restarts with new paths - TrainingPitPanel: ActivateAdapterRequested event; "Load to Runtime" card auto-shows after training completes; ModelDepotRoot property for GGUF discovery Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs (1)
1231-1271: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFragile
SelectedIndex-based routing for target/backend.
_genTargetIsToolcaller = CbGenTarget.SelectedIndex == 1;and the backendswitch (CbGenBackend.SelectedIndex)hardcode positional assumptions about combobox item order. Elsewhere in this file (CbHarvestDuration) pickers use aTagon eachComboBoxItemprecisely to avoid this coupling — a future reorder/insert in the XAML silently reroutes generation to the wrong target/backend with no compile-time or runtime signal.♻️ Suggested approach
- _genTargetIsToolcaller = CbGenTarget.SelectedIndex == 1; + _genTargetIsToolcaller = (CbGenTarget.SelectedItem as ComboBoxItem)?.Tag as string == "toolcaller";and similarly read
_genBackendfrom(CbGenBackend.SelectedItem as ComboBoxItem)?.Tag.🤖 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 `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs` around lines 1231 - 1271, The target/backend selection logic in CbGenTarget_SelectionChanged and CbGenBackend_SelectionChanged is brittle because it depends on SelectedIndex ordering. Update both handlers to read the chosen ComboBoxItem’s Tag (like the existing CbHarvestDuration pattern) and derive _genTargetIsToolcaller and _genBackend from those tags instead of hardcoded index positions, so item reordering in XAML cannot silently change behavior.OrchestratorIDE/Core/LlamaServerManager.cs (1)
242-245: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing LoRA silently skipped with no log message.
ModelPathgets a hard failure +Log(...)inStartAsyncwhen the file is missing, but a configured-but-missingLoraPathjust silently omits--lorawith zero feedback. After a Foundry "Activate" that points at a moved/deleted adapter file, the server starts fine on the base model only, and the user has no indication their adapter never loaded.🔧 Suggested fix
- if (!string.IsNullOrWhiteSpace(LoraPath) && File.Exists(LoraPath)) - sb.Append($" --lora \"{LoraPath}\""); + if (!string.IsNullOrWhiteSpace(LoraPath)) + { + if (File.Exists(LoraPath)) + sb.Append($" --lora \"{LoraPath}\""); + else + Log($"LoRA adapter configured but not found, starting without it: {LoraPath}"); + }🤖 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 `@OrchestratorIDE/Core/LlamaServerManager.cs` around lines 242 - 245, The LoRA adapter path is being skipped silently when `LoraPath` is configured but the file is missing. Update `LlamaServerManager` in the `StartAsync`/command-building flow so it logs a clear warning or error when `LoraPath` is set but `File.Exists(LoraPath)` is false, similar to the existing `ModelPath` handling, instead of only omitting the `--lora` argument. Use the existing `Log(...)` pattern and the `LoraPath` check near the LoRA adapter block to locate the fix.OrchestratorIDE.Avalonia/MainWindow.axaml.cs (2)
2077-2099: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFoundry activation silently overwrites the main runtime model path with no backup/confirmation.
ApplyFoundryAdapterunconditionally replaces_settings.LlamaCppModelPath/LlamaCppLoraPathwith the small toolcaller specialist's base/adapter GGUF and persists it immediately. If the user was previously using llama.cpp for their primary chat/swarm model (a different, larger GGUF), that configuration is overwritten with no way to recover the prior path other than manually re-entering it in Settings. Worth considering a confirmation prompt or preserving the previous model path for one-click revert, since this button repurposes the app's single shared inference slot rather than a scoped "toolcaller sandbox" runtime.🤖 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 `@OrchestratorIDE.Avalonia/MainWindow.axaml.cs` around lines 2077 - 2099, ApplyFoundryAdapter currently overwrites the shared llama.cpp model and LoRA settings without preserving the previous runtime configuration. Update this flow so the existing LlamaCppModelPath/LlamaCppLoraPath values are backed up or the user is prompted to confirm before Save is called, and add a clear way to restore the prior model after the Foundry adapter is loaded. Keep the logic localized to ApplyFoundryAdapter and the surrounding Settings/ActivityEvent handling so the single shared inference slot is not permanently replaced without user consent.
285-289: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
ModelDepotRootis never refreshed after construction.
_pitPanel.ModelDepotRootis set once here from_settings.ResolvedModelStoragePath. UnlikeWorkspaceRoot/OllamaHost, which get re-pushed into_pitPanelinSetMode("pit"), there's no analogous update path forModelDepotRoot. If the user changes "Model Storage Path" in Settings after startup, the Foundry gate card's GGUF discovery (FindFoundryLoraGguf/FindFoundryBaseGguf) keeps searching the stale original path until the app restarts.🔧 Suggested fix
else if (mode == "pit") { _pitPanel.WorkspaceRoot = _session.WorkspaceRoot; _pitPanel.OllamaHost = _settings.OllamaHost; + _pitPanel.ModelDepotRoot = _settings.ResolvedModelStoragePath; _pitPanel.Refresh();🤖 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 `@OrchestratorIDE.Avalonia/MainWindow.axaml.cs` around lines 285 - 289, `TrainingPitPanel.ModelDepotRoot` is only initialized in `MainWindow` and never refreshed, so it can keep using a stale model storage path after settings change. Update the same place that re-pushes `WorkspaceRoot` and `OllamaHost` in `SetMode("pit")` to also assign `_pitPanel.ModelDepotRoot` from the latest `_settings.ResolvedModelStoragePath`, and ensure any settings-change path that affects model storage also refreshes the panel so `FindFoundryLoraGguf` and `FindFoundryBaseGguf` search the current location.
🤖 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 `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs`:
- Around line 1650-1683: The Activate GGUF lookup in
`RefreshFoundryActivateCard`, `FindFoundryLoraGguf`, `FindFoundryBaseGguf`, and
the `BtnFoundryActivate_Click` flow can throw if `ModelDepotRoot` or
`FoundryOutDir` is missing or disappears between refresh and click. Add a guard
before enumerating files or wrap the lookup in a safe failure path so the click
handler checks directory existence and handles null results gracefully. Keep the
UI state in sync by only enabling activation when both GGUF paths are confirmed
available, and avoid any uncaught exceptions from the file search helpers.
In `@training_pit/foundry/scripts/conversion/__init__.py`:
- Around line 342-350: Guard the registry lookup in get_model_class so
conditionally unavailable models do not escape as KeyError; after the dynamic
import and before returning from ModelBase._model_classes, verify the requested
name exists in the selected registry and convert any missing entry into the same
NotImplementedError path used for unsupported architectures. Keep the fix inside
get_model_class and preserve the existing ModelType selection for mmproj versus
text models.
---
Nitpick comments:
In `@OrchestratorIDE.Avalonia/MainWindow.axaml.cs`:
- Around line 2077-2099: ApplyFoundryAdapter currently overwrites the shared
llama.cpp model and LoRA settings without preserving the previous runtime
configuration. Update this flow so the existing
LlamaCppModelPath/LlamaCppLoraPath values are backed up or the user is prompted
to confirm before Save is called, and add a clear way to restore the prior model
after the Foundry adapter is loaded. Keep the logic localized to
ApplyFoundryAdapter and the surrounding Settings/ActivityEvent handling so the
single shared inference slot is not permanently replaced without user consent.
- Around line 285-289: `TrainingPitPanel.ModelDepotRoot` is only initialized in
`MainWindow` and never refreshed, so it can keep using a stale model storage
path after settings change. Update the same place that re-pushes `WorkspaceRoot`
and `OllamaHost` in `SetMode("pit")` to also assign `_pitPanel.ModelDepotRoot`
from the latest `_settings.ResolvedModelStoragePath`, and ensure any
settings-change path that affects model storage also refreshes the panel so
`FindFoundryLoraGguf` and `FindFoundryBaseGguf` search the current location.
In `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs`:
- Around line 1231-1271: The target/backend selection logic in
CbGenTarget_SelectionChanged and CbGenBackend_SelectionChanged is brittle
because it depends on SelectedIndex ordering. Update both handlers to read the
chosen ComboBoxItem’s Tag (like the existing CbHarvestDuration pattern) and
derive _genTargetIsToolcaller and _genBackend from those tags instead of
hardcoded index positions, so item reordering in XAML cannot silently change
behavior.
In `@OrchestratorIDE/Core/LlamaServerManager.cs`:
- Around line 242-245: The LoRA adapter path is being skipped silently when
`LoraPath` is configured but the file is missing. Update `LlamaServerManager` in
the `StartAsync`/command-building flow so it logs a clear warning or error when
`LoraPath` is set but `File.Exists(LoraPath)` is false, similar to the existing
`ModelPath` handling, instead of only omitting the `--lora` argument. Use the
existing `Log(...)` pattern and the `LoraPath` check near the LoRA adapter block
to locate the fix.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 181ffb68-9819-4030-a972-35328b80dcf5
📒 Files selected for processing (9)
OrchestratorIDE.Avalonia/MainWindow.axaml.csOrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axamlOrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.csOrchestratorIDE/Core/AppSettings.csOrchestratorIDE/Core/LlamaServerManager.cstraining_pit/foundry/scripts/conversion/__init__.pytraining_pit/foundry/scripts/conversion/base.pytraining_pit/foundry/scripts/conversion/qwen.pytraining_pit/foundry/scripts/convert_lora_to_gguf.py
🚧 Files skipped from review as they are similar to previous changes (1)
- OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml
Comment on lines
+1650
to
+1683
| private void RefreshFoundryActivateCard() | ||
| { | ||
| var loraGguf = FindFoundryLoraGguf(); | ||
| var baseGguf = FindFoundryBaseGguf(); | ||
| var visible = loraGguf is not null && baseGguf is not null; | ||
| BorderFoundryActivate.IsVisible = visible; | ||
| if (visible) | ||
| TbFoundryActivateInfo.Text = | ||
| $"Adapter ready: {Path.GetFileName(loraGguf)}\n" + | ||
| $"Base model: {Path.GetFileName(baseGguf)}"; | ||
| } | ||
|
|
||
| private string? FindFoundryLoraGguf() | ||
| { | ||
| // Prefer the depot, fall back to workspace outputs | ||
| var depotLora = string.IsNullOrEmpty(ModelDepotRoot) ? null | ||
| : Directory.EnumerateFiles(ModelDepotRoot, "theorc-toolcaller-*lora*.gguf") | ||
| .OrderByDescending(File.GetLastWriteTimeUtc) | ||
| .FirstOrDefault(); | ||
| if (depotLora is not null) return depotLora; | ||
|
|
||
| return Directory.EnumerateFiles(FoundryOutDir, "*lora*.gguf", SearchOption.AllDirectories) | ||
| .OrderByDescending(File.GetLastWriteTimeUtc) | ||
| .FirstOrDefault(); | ||
| } | ||
|
|
||
| private string? FindFoundryBaseGguf() | ||
| { | ||
| if (string.IsNullOrEmpty(ModelDepotRoot)) return null; | ||
| return Directory.EnumerateFiles(ModelDepotRoot, "Qwen2.5-1.5B*.gguf") | ||
| .OrderByDescending(File.GetLastWriteTimeUtc) | ||
| .FirstOrDefault(); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file structure first
ast-grep outline OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs --view expanded || true
# Inspect the target region and the activate click handler
sed -n '1600,2045p' OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs | cat -n
# Find where these helpers are used elsewhere
rg -n "FindFoundryLoraGguf|FindFoundryBaseGguf|RefreshFoundry\(|BtnFoundryActivate_Click|ModelDepotRoot|FoundryOutDir" OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.csRepository: hardcoreerik/TheOrc
Length of output: 38655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the full RefreshFoundry method and the activate handler context
sed -n '1581,1655p' OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs | cat -n
# Look for where the recipe config/base model comes from
rg -n "base_model|hf_repo|toolcaller_v0.json|MapOllamaToHfRepo|FoundryConfigPath|RefreshFoundry\(" OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.csRepository: hardcoreerik/TheOrc
Length of output: 5432
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# See whether RefreshFoundry is called inside any broader exception handling
sed -n '280,340p' OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs | cat -n
# Inspect the recipe/config that defines the foundry track
git ls-files | rg 'toolcaller_v0\.json|THEORC_FOUNDRY|foundry.*json|toolcaller'Repository: hardcoreerik/TheOrc
Length of output: 3654
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the toolcaller recipe for the declared base model
sed -n '1,220p' training_pit/foundry/configs/toolcaller_v0.json | cat -n
# Check whether the activate UI is gated by visibility or command wiring elsewhere
rg -n "FoundryActivate|BtnFoundryActivate|BorderFoundryActivate|ActivateAdapterRequested" OrchestratorIDE.Avalonia/UI -g '!**/bin/**' -g '!**/obj/**'Repository: hardcoreerik/TheOrc
Length of output: 6203
Guard the Activate GGUF lookup (TrainingPitPanel.axaml.cs:1662-1679, 1991-2003) BtnFoundryActivate_Click calls FindFoundryLoraGguf()/FindFoundryBaseGguf() directly, and either helper can throw if the depot/output directory is missing or disappears between refresh and click. Add an existence check or wrap the lookup so Activate fails gracefully instead of surfacing an unhandled UI exception.
🤖 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 `@OrchestratorIDE.Avalonia/UI/Panels/TrainingPitPanel.axaml.cs` around lines
1650 - 1683, The Activate GGUF lookup in `RefreshFoundryActivateCard`,
`FindFoundryLoraGguf`, `FindFoundryBaseGguf`, and the `BtnFoundryActivate_Click`
flow can throw if `ModelDepotRoot` or `FoundryOutDir` is missing or disappears
between refresh and click. Add a guard before enumerating files or wrap the
lookup in a safe failure path so the click handler checks directory existence
and handles null results gracefully. Keep the UI state in sync by only enabling
activation when both GGUF paths are confirmed available, and avoid any uncaught
exceptions from the file search helpers.
Comment on lines
+342
to
+350
| def get_model_class(name: str, mmproj: bool = False) -> Type[ModelBase]: | ||
| """Dynamically import and return a model class by its HuggingFace architecture name.""" | ||
| relevant_map = MMPROJ_MODEL_MAP if mmproj else TEXT_MODEL_MAP | ||
| if name not in relevant_map: | ||
| raise NotImplementedError(f"Architecture {name!r} not supported!") | ||
| module_name = relevant_map[name] | ||
| __import__(f"conversion.{module_name}") | ||
| model_type = ModelType.MMPROJ if mmproj else ModelType.TEXT | ||
| return ModelBase._model_classes[model_type][name] |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C4 '`@ModelBase`\.register' training_pit/foundry/scripts/conversion/ | rg -nP -B4 'if .*is not None|try:|except'Repository: hardcoreerik/TheOrc
Length of output: 1290
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## __init__.py around get_model_class\n'
sed -n '300,370p' training_pit/foundry/scripts/conversion/__init__.py
printf '\n## qwen.py around DFlash registration\n'
sed -n '620,645p' training_pit/foundry/scripts/conversion/qwen.py
printf '\n## convert_lora_to_gguf.py around exception handling\n'
sed -n '390,415p' training_pit/foundry/scripts/conversion/convert_lora_to_gguf.py
printf '\n## mapped architectures that reference qwen.py\n'
rg -n '"qwen"' training_pit/foundry/scripts/conversion/__init__.py training_pit/foundry/scripts/conversion/qwen.pyRepository: hardcoreerik/TheOrc
Length of output: 3925
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## mapped names that resolve to qwen.py\n'
rg -n '"qwen"' training_pit/foundry/scripts/conversion/__init__.py
printf '\n## all conditional `@ModelBase.register` sites in conversion/\n'
rg -n -C3 '`@ModelBase`\.register' training_pit/foundry/scripts/conversion/ | rg -n -C3 'try:|except AttributeError|if .* is not None|if .*!= None|if .* is not None'
printf '\n## call sites catching NotImplementedError or KeyError around get_model_class\n'
rg -n -C3 'get_model_class\(|except (NotImplementedError|KeyError)|except NotImplementedError|except KeyError' training_pit/foundry/scripts/conversion
printf '\n## locate convert_lora_to_gguf.py\n'
fd -a 'convert_lora_to_gguf.py' training_pit/foundry/scripts/conversion .Repository: hardcoreerik/TheOrc
Length of output: 3120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## convert_lora_to_gguf.py around the get_model_class call\n'
sed -n '390,415p' /home/jailuser/git/training_pit/foundry/scripts/convert_lora_to_gguf.py
printf '\n## all get_model_class call sites and nearby exception handling\n'
rg -n -C3 'get_model_class\(' /home/jailuser/git/training_pit/foundry/scripts
rg -n -C3 'except (NotImplementedError|KeyError)|except NotImplementedError|except KeyError' /home/jailuser/git/training_pit/foundry/scripts/convert_lora_to_gguf.py /home/jailuser/git/training_pit/foundry/scripts/conversionRepository: hardcoreerik/TheOrc
Length of output: 4655
Guard the registry lookup in get_model_class get_model_class() can still raise KeyError for conditionally registered models (for example DFlashDraftModel when gguf.MODEL_ARCH.DFLASH is absent). The caller only handles NotImplementedError, so this escapes as an uncaught crash instead of a clean “not supported” error.
🤖 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/conversion/__init__.py` around lines 342 - 350,
Guard the registry lookup in get_model_class so conditionally unavailable models
do not escape as KeyError; after the dynamic import and before returning from
ModelBase._model_classes, verify the requested name exists in the selected
registry and convert any missing entry into the same NotImplementedError path
used for unsupported architectures. Keep the fix inside get_model_class and
preserve the existing ModelType selection for mmproj versus text models.
hardcoreerik
added a commit
that referenced
this pull request
Jul 13, 2026
* Arena benchmark panel (Stage 4) + pitRoot resolver fix (#43) Adds the full ARENA evaluation panel to Training Pit Stage 4: - eval_toolcaller.py: PEFT adapter eval script with live progress.json polling; decision accuracy, JSON validity, tool precision, arg exact match, per-class F1 for call/no_tool/clarify/unsupported - TrainingPitPanel: Arena section with adapter picker, run/stop/folder buttons, live progress bar, and metric tiles that render as results stream in every 3 seconds - ResolvePitRoot now requires training_pit/foundry to exist so workspace temp dirs with stub training_pit/datasets no longer shadow the real repo - foundry_preflight leakage check: hashes (user_prompt, assistant) pairs instead of assistant strings alone to avoid false positives Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * theorc-toolcaller: name, deploy, and wire the first Foundry specialist (#43) Kill gate PASSED — trained adapter vs untuned base on the sealed 260-example held-out set: decision accuracy 63.9% -> 97.3%, tool precision 63.0% -> 96.2%, unsupported F1 0.000 -> 1.000 (base model never refuses; the specialist always does when it should). Full snapshot in docs/THEORC_TOOLCALLER_V0_BASELINE.md. - Register foundry_toolcaller_v0_r2 as theorc-toolcaller:qwen25-1.5b (GGUF LoRA f16 over qwen2.5:1.5b-instruct; modelfile follows pitboss pattern) - ToolcallerService: runtime client with prompt serialization locked byte-identical to export_toolcaller_dataset.py, defensive decision parsing, invented-tool rejection; 15 new unit tests (20/20 toolcaller suite green) - Opt-in repair lane in SwarmSession.RunWorkerLoopAsync: when a worker states intent but emits no parseable call, the specialist proposes one; the proposal re-enters the normal loop so ToolPolicyEngine + approval stay authoritative. Default OFF (ToolcallerRepairEnabled) per the v0 spec; works on HIVE nodes via the same client branch the worker used, silent fallback when absent - Fix Arena progress race that killed the first baseline run at 20/260: eval_toolcaller.py retries the atomic tmp->json replace instead of dying, and the Training Pit poller reads with FileShare.ReadWrite|Delete Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Refusal Gauntlet: adversarial refusal coverage for the Foundry (#43) Answers "14/14 unsupported is not A-to-Z coverage": 4,788 deterministic adversarial cases (6 families x 798, seed 43, no teacher model), scored with exact one-sided 95% Clopper-Pearson lower bounds and paraphrase-flip detection, run against the DEPLOYED Ollama artifact. First run vs theorc-toolcaller:qwen25-1.5b (2026-07-12): safety rate 91.5% (cp95 >= 90.8%) - 407 fabricated calls of 4,788 strict accuracy 57.7% - 426/798 paraphrase groups flip HEADLINE: missing_argument strict = 0.0% - the model answers no_tool 600x and fabricates 169x on vague requests, never clarify, despite 0.918 clarify-F1 on the Arena set. Invisible without an adversarial distribution; failures.jsonl is the pre-labeled r3 training backlog. - generate_refusal_gauntlet.py: template-combinatorial generator, prompt serialization imported from the exporter (training-identical by construction); dataset reproducible from seed, not committed (gitignored) - eval_refusal_gauntlet.py: --ollama or --adapter backends, two-tier strict/safety scoring, exact CP lower bounds (no scipy), flip groups, failures.jsonl, 4-worker concurrency, atomic progress writes - Training Pit: Refusal Gauntlet row inside Stage 4 ARENA - run/stop, live progress, safety badge, per-family bars (fabrication-risk families in red) - docs/TOOLCALLER_REFUSAL_GAUNTLET.md: methodology, why bare-LLM five nines is the wrong claim, system-level nines argument, retraining loop Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * toolcaller r3: gauntlet failures retrained, both gates green (#43) Closes the refusal-gauntlet loop: fold r2 failures back into training with group-level contamination guards, retrain, re-run both gates. - build_toolcaller_r3_dataset.py: 70/30 paraphrase-group hash split; 900 rows from failing train-side groups join the untouched r2 set (1,903 total, call still 32%); 231 held-out groups (1,386 rows) become the re-eval set - no group crosses sides. Honest meta sidecar (no forged validator PASS; trained with --skip-gates, reason in run manifest) - r3 trained 27 min, eval_loss 0.0799 (r2 0.0831); deployed as theorc-toolcaller:qwen25-1.5b-r3 (r2 tag untouched pending promotion) Gate 2, gauntlet holdout (never-trained groups), r2 -> r3: strict 54.9% -> 96.3% (cp95>=95.4) safety 90.3% -> 98.3% (134 -> 24 fabrications) missing_argument 0%/72% -> 91.1%/100% flips 182 -> 19 groups Gate 1, sealed Arena 260 (regression): 97.3% -> 98.5% decision acc, tool precision 96.2% -> 97.8%, clarify F1 0.918 -> 0.966 - refusal data HELPED call behavior. Remaining r4 backlog: benign_no_tool 99.6% -> 89.3% (all surviving fabrications are over-eager calls on ordinary chat; counter-swing). Also: eval_toolcaller.py utf-8 console reconfigure (cp1252 crash in the final summary print, after results.json was safely written). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Promote toolcaller r3 to the main tag + freeze the promotion margin (#43) - toolcaller-qwen25-1.5b.modelfile now builds from the r3 adapter: both gates green (Arena 97.3->98.5 decision acc, gauntlet holdout safety 90.3->98.3, missing_argument fabrications eliminated). Rollback = repoint ADAPTER at the r2 output dir and rebuild the tag. - Freeze the promotion margin in toolcaller_v0_r3.json per the spec rule that it must predate the candidates it judges: governs r4+, keyed on the gauntlet-holdout safety cp95 lower bound strictly improving, Arena decision accuracy dropping at most 1 point, and no per-family safety regression. Scope note records that r3 itself predates the rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Review fixes: machine-readable incumbent bar + frozen group-split guard (#44) From the efficient-diff-reviewer pass: - promotion.margin.incumbent now carries the r3 numbers as data (0.9757 safety cp95 lower, 0.9846 arena acc) instead of prose, so a future gate script can enforce the rules instead of trusting review - group split (holdout_frac=0.30, seed=7) frozen alongside the margin, and build_toolcaller_r3_dataset.py now hard-refuses to run with different values - held-out gauntlet groups can no longer silently migrate train-side via a parameter change Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * CodeRabbit fixes: repair-lane capture guard, GPU mutex, portability (#44) All 13 findings triaged; the two that mattered most: - CONTAMINATION GUARD: repair-lane proposals were staged by ToolcallerDatasetCapture as "organic" examples - the specialist would have fed its own outputs into its next training round. StageCallAsync is now skipped for calls carrying ToolcallerService.RepairProvenanceMarker. - GPU mutex completed in both directions: Arena now refuses when gen/harvest/review/gauntlet own the GPU; Gauntlet takes the full mutex; Forge/Foundry/harvest/gen/review launchers and the review-button gpuFree check all refuse when Arena or Gauntlet is running. Also: - Arena/Gauntlet launchers follow the cross-platform cmd.exe|/bin/sh pattern via a shared BuildPythonEvalLauncher (was Windows-only cmd.exe) - eval_refusal_gauntlet: malformed rows are skipped+counted instead of aborting the pool; trust_remote_code dropped (Qwen2.5 is native since transformers 4.37) - eval_toolcaller: empty-results summary guard; trust_remote_code dropped; tool_precision key documented as correct-tool rate over expected calls (kept for cross-run comparability, console label fixed) - meta sidecar records repo-relative source paths (was machine-absolute); regenerated - dataset files verified byte-identical - stale r2 numbers scrubbed from SettingsPanel hint, AppSettings/ ToolcallerService XML docs, and the r3 modelfile header (docs now point at TOOLCALLER_REFUSAL_GAUNTLET.md instead of embedding rot-prone figures) 20/20 toolcaller tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix temp launcher script leak on non-Windows (#44) CodeRabbit re-review, second pass: both new BuildPythonEvalLauncher call sites (Arena, Gauntlet) discarded the /bin/sh tempScript path via out _, so it was never deleted on Linux/macOS. Now captured and wired to Process.Exited cleanup, matching StartForge/BtnGenStart_Click/ BtnFoundryTrain_Click's existing pattern -- including cleanup on the launch-failure paths, which those three don't need (their scripts are written before Process.Start can fail) but these do. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
5 tasks
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.
Summary
New backend:
training_pit/foundry/scripts/generate_toolcaller_dataset.py— seeded scenario generator producing balanced toolcaller-v0 capture JSON files. Decision type is predetermined by recipe (40% call / 30% no_tool / 20% clarify / 10% unsupported across 4 roles); Ollama only fills in realistic request text and arguments, preventing label drift. Outputs totraining_pit/datasets/toolcaller/ready forToolcallerBench+export_toolcaller_dataset.py.Stage 1 target switcher: Generate Dataset now has a "Generation Target" picker — Boss Plans → ORC ACADEMY (existing) or Toolcaller Examples → THE FOUNDRY (new). Switching target auto-updates the key field, hint text, and which script is invoked. After a successful toolcaller run the "→ Captures ready — validate in THE FOUNDRY" button appears and triggers
RefreshFoundry().Stage 3 gate card: THE FOUNDRY now shows a live training-readiness card with color-coded bars tracking staged/accepted counts toward the 150-capture gate and a per-dataset ETA estimate (Qwen2.5-1.5B LoRA, ~2–8 min).
Route labels:
-> Stage 2andswarm capturesbadges added to Stage 1 and Stage 3 column headers so the two distinct data paths are visually unambiguous.Full workflow enabled
Test plan
generate_toolcaller_dataset.py --count 10: produces 10 valid capture JSON files, allreview_status: pending🤖 Generated with Claude Code
Summary by CodeRabbit