CF-7 test-harness diagnostics + native CUDA backend fixes (dev + release + installer) - #39
Merged
Merged
Conversation
Explains BuildEvidencePack/BuildTopKText/BuildExhaustiveAnswer evidence selection, FabricAnswerVerifier grading rules, and the JSON recovery pipeline, so the next 120-question gate result can be judged against documented behavior rather than re-derived from commit archaeology. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Records the HARDCOREPC native-library load regression (confirmed model-independent, not yet root-caused) and the Windows/OpenSSH process-detachment gotcha discovered while setting up fleet-wide runs, so a future failure on that box isn't mistaken for a scoring bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…scores The 2026-07-04 full 120-question run's B3=12/120 and B0 failures were traced to native NoKvSlot crashes (216 of 223 B3 failures), not real verification failures -- the BuildEvidencePack fix's uncapped evidence packs (up to 26 segments/6.3K tokens per question) exhaust the KV-cache pool faster than AdapterManager's conversation-count-based recycle threshold accounts for. Root-caused via the raw result JSON and the existing recycle-logic comments in AdapterManager.cs; not yet fixed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BuildEvidencePack's uncapped evidence packs (up to ~26 segments per question vs 1-4 before) exhaust the shared KV-cache pool well under the old 128-conversation recycle threshold, since disposed conversations' KV memory is never reclaimed by llama.cpp -- confirmed via the 2026-07-04 CF-7 run's NoKvSlot failures. Tightening to 24 is a conservative, zero-logic-change stopgap while a real token-based recycle trigger is designed and tested; not yet validated against a full 120-question run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nations The reducer-validation gate (ContextFabricFeasibilityRunner.cs:515) correctly rejected Gemma-4-12B claim-ID inventions in 6 cases -- this is the harness's honesty check working as intended, distinct from the NoKvSlot infrastructure noise documented above it, and not something to change in the scoring logic. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A second full 120-question run with SequenceRecycleThreshold lowered 128->24 produced a byte-for-byte identical failure trace to the pre-fix run. That rules out conversation-count as the actual gating mechanism (or recycling isn't firing at all) -- retracting the earlier "stopgap applied" framing in favor of an honest account of what was tried, what the evidence shows, and the narrower hypothesis (a stuck ActiveCount blocking the recycle branch entirely) that needs real debugging to confirm, not another guessed constant change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…gation Two single-constant threshold changes have now been tried against the CF-7 gate's KV-cache exhaustion bug with no effect on the failure trace, which points at ActiveCount possibly never reaching zero rather than the threshold value itself. Rather than guess a third time, add a purely additive, opt-in diagnostic (THEORC_KVCACHE_DIAGNOSTICS=1) that logs every recycle-eligibility decision to stderr -- zero behavior change unless explicitly enabled, so it's safe to ship even though the underlying bug isn't fixed yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Discovered immediately when actually run: Run-CF7GateExpanded.ps1 pipes the benchmark exe through 2>&1 | Tee-Object, and PowerShell treats any native-process stderr line as a terminating NativeCommandError, killing the whole run after just the first diagnostic line. stdout avoids the collision entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two real gaps, found while investigating HARDCOREPC's native-library load regression: FormatLoadFailure only showed one level of InnerException, truncating exactly the detail an exception chain like TypeInitializationException -> RuntimeError -> (real cause) needs; and NativeBackendBootstrap.EnsureConfigured()'s returned report (CUDA driver detection, cuda12 DLL pre-flight results, selected backend) was computed and then thrown away at every call site. Both fixed: FormatLoadFailure walks the full chain, and a load failure now appends the backend report's verdict and log lines to the error message. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…talled Root-caused via LLamaSharp's own diagnostic log (finally visible after the previous commit stopped discarding it): NativeLibraryWithCuda only falls back to trying the packaged cuda12 (then cuda11) folder when its majorCudaVersion is exactly -1 -- the "no toolkit, driver only" case this fleet was built around. When a real CUDA toolkit is present (e.g. after installing CUDA 13.3 for unrelated dev work), LLamaSharp's own toolkit detection succeeds and returns that version instead, and the class then ONLY tries that one exact version's folder with zero fallback -- so a machine with a newer/different toolkit installed than whatever we've packaged fails outright, even though the working cuda12 backend is right there. Confirmed live on HARDCOREPC: installing CUDA 13.3 broke native loading entirely, with the log showing repeated attempts against a nonexistent "cuda13" folder and never touching cuda12. Fixed with a small custom INativeLibrarySelectingPolicy (Cuda12FallbackSelectingPolicy) that forces CUDA candidates back to majorCudaVersion=-1 regardless of what toolkit LLamaSharp detects -- using only public LLamaSharp APIs (WithSelectingPolicy), no internal hacks. Makes the app work with or without a CUDA toolkit installed, and regardless of which version, as long as the driver is CUDA-capable and our packaged cuda12 backend is compatible with it (CUDA maintains strong runtime backward compatibility, so this holds for any reasonably current driver). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found while investigating HARDCOREPC's native-library regression: OrchestratorIDE.NativeRuntime.csproj sources cudart64_12.dll, cublas64_12.dll, and cublasLt64_12.dll from whatever CUDA Toolkit happens to be on the BUILD machine (TheOrcCudaRedistDir/CUDA_PATH) -- the LLamaSharp.Backend.Cuda12.Windows NuGet package does not ship them itself. The actual release workflow builds on a stock windows-latest GitHub-hosted runner with no CUDA Toolkit and no GPU, so every official release build has been silently missing these DLLs, meaning the shipped app CPU-falls-back for every real end user with an NVIDIA GPU -- the exact bug already found once on a fleet dev machine (see the existing NativeRuntime.csproj comment) but never fixed at the release level. Adds Tools/Get-CudaRedistributables.ps1, which fetches just the ~3 files needed directly from NVIDIA's own official redistributable manifest feed (the same channel conda/pip use for their nvidia-cuda-runtime-cu12/nvidia-cublas-cu12 packages), SHA-256 verified against NVIDIA's published manifest -- no full toolkit install, no third-party NuGet repackaging. Verified locally: manifest fetch, checksum verification, extraction, and MSBuild pickup via TheOrcCudaRedistDir all confirmed working end to end. Wired into release.yml's Windows leg before the OrchestratorIDE publish step. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Completes the release-side fix alongside the CI-side one (Tools/Get-CudaRedistributables.ps1, release.yml): the release build now bundles cudart64_12.dll/cublas64_12.dll/cublasLt64_12.dll for every Windows build, but baking them unconditionally into every download bloats it for AMD/Intel/CPU-only users who'll never touch the CUDA path. Since OrchestratorIDE.exe publishes as a self-extracting single-file bundle, its actual runtime working directory is a dynamically-generated temp extraction folder that the installer can't predict or write into before the app has ever launched -- so an installer-placed file next to the exe wouldn't be found by the app's own bundle-relative native-library probing. Fixed with a stable, installer-controlled location (%LOCALAPPDATA%\TheOrc\CudaRedist, outside any install/extraction path) plus a small addition to NativeBackendBootstrap's existing preflight: it now pre-loads the three redistributables from this stable directory (via absolute-path NativeLibrary.TryLoad, same mechanism the preflight already uses for the bundle's own DLLs) when the bundle-relative cuda12 folder doesn't already have them. Windows resolves a loaded DLL's dependency imports against already-loaded same-named modules first, so ggml-cuda.dll's import of cudart64_12.dll resolves correctly regardless of which of the two directories actually supplied it. CudaRedistributableInstaller mirrors the PowerShell script's NVIDIA official-manifest approach (no toolkit install, no third-party NuGet repackaging) but reuses this project's own DownloadService (resumable, SHA-256-verified, retrying) and ZipExtractService (zip-slip-guarded) rather than reimplementing HTTP/hashing/extraction a second time. Gated on OperatingSystem.IsWindows() && DetectedGpuVendor == "nvidia" so other installs never pay for a download they can't use. Verified end-to-end with a standalone harness: manifest fetch, SHA-256 verification, extraction, and idempotent re-run (skips already-present files) all confirmed working. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds CUDA redistributable fetching and installation across release, setup, and runtime paths, plus enhanced model-load and KV-cache diagnostics and expanded Context Fabric documentation. ChangesCUDA Redistributables and Runtime Diagnostics
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 3
🧹 Nitpick comments (3)
Tools/Get-CudaRedistributables.ps1 (2)
90-90: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider suppressing the progress bar for large downloads.
Invoke-WebRequestwithout$ProgressPreference = 'SilentlyContinue'can render a progress UI that significantly slows large downloads on some PowerShell hosts. Since this fetches multi-MB CUDA redistributable archives on every cache-miss CI run, this is worth a quick win.Also applies to: 90-90
🤖 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 `@Tools/Get-CudaRedistributables.ps1` at line 90, Suppress the PowerShell progress UI around the download in Get-CudaRedistributables so large archive fetches don’t slow CI; update the download logic that uses Invoke-WebRequest to temporarily set $ProgressPreference to SilentlyContinue before the request and restore it afterward, keeping the behavior localized to this script.
89-116: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo try/catch around
Expand-Archive/Copy-Item; unhandled terminating errors may not reliably produce a clean non-zero signal for the caller.
$ErrorActionPreference = "Stop"(Line 32) means these calls throw terminating errors on failure, but unlike the manifest fetch (Lines 65-70) and download (Lines 89-94), there's no surroundingtry/catchhere to guarantee a controlledexit 1. Combined with theWrite-Host/pipeline issue above, relying on implicit script termination behavior for CI failure detection is fragile.♻️ Suggested hardening
- $extractDir = Join-Path $workDir ([System.IO.Path]::GetFileNameWithoutExtension($relativePath)) - Expand-Archive -Path $zipPath -DestinationPath $extractDir -Force + $extractDir = Join-Path $workDir ([System.IO.Path]::GetFileNameWithoutExtension($relativePath)) + try { + Expand-Archive -Path $zipPath -DestinationPath $extractDir -Force + } catch { + Write-Host "Failed to extract $component : $($_.Exception.Message)" -ForegroundColor Red + exit 1 + }🤖 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 `@Tools/Get-CudaRedistributables.ps1` around lines 89 - 116, The archive extraction and DLL copy path in Get-CudaRedistributables.ps1 is missing explicit error handling, so failures from Expand-Archive or Copy-Item may not surface as a clean controlled exit. Wrap the extraction and file-copy loop in a try/catch like the existing download handling, and on any exception Write-Host a clear failure message including the caught error and exit 1. Keep the fix localized around the Expand-Archive, Get-ChildItem, and Copy-Item logic so the script always returns a reliable non-zero signal to callers.OrchestratorSetup/Services/CudaRedistributableInstaller.cs (1)
71-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDispose the
JsonDocument.
JsonDocument.Parse(...)returns anIDisposablebacked by pooled buffers; here only.RootElementis retained and the document is never disposed, so the rented buffers are never returned to the pool. Sincemanifestis consumed later in theforeach, keep the document alive for that scope with ausing.♻️ Proposed fix
- JsonElement manifest; + using JsonDocument manifestDoc; using (var http = new HttpClient()) { http.DefaultRequestHeaders.UserAgent.ParseAdd("OrchestratorSetup/1.0"); var manifestUrl = $"{RedistBaseUrl}/redistrib_{ManifestVersion}.json"; Log($"Fetching NVIDIA CUDA redistributable manifest ({ManifestVersion})..."); var manifestJson = await http.GetStringAsync(manifestUrl, ct); - manifest = JsonDocument.Parse(manifestJson).RootElement; + manifestDoc = JsonDocument.Parse(manifestJson); } + var manifest = manifestDoc.RootElement;🤖 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 `@OrchestratorSetup/Services/CudaRedistributableInstaller.cs` around lines 71 - 79, Dispose the JsonDocument created in CudaRedistributableInstaller when parsing the manifest, because only RootElement is kept and the pooled buffers are otherwise leaked. Update the manifest-loading block in the install flow to keep the JsonDocument alive for the full scope where manifest is later used in the foreach, using a using declaration or using statement around JsonDocument.Parse so the document is released after all manifest access is complete.
🤖 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 `@docs/CONTEXT_FABRIC_TEST_HARNESS.md`:
- Around line 312-321: The diagnostic stream description is incorrect:
`THEORC_KVCACHE_DIAGNOSTICS=1` in `AdapterManager.cs` writes the
recycle-eligibility lines to stdout, not stderr. Update the runbook text in
`CONTEXT_FABRIC_TEST_HARNESS.md` to match the actual behavior and keep the
explanation aligned with the `AdapterManager` diagnostic output so operators
know PowerShell will not treat it as a `NativeCommandError`.
In `@OrchestratorSetup/Services/CudaRedistributableInstaller.cs`:
- Around line 49-56: The XML summary on
CudaRedistributableInstaller.EnsureCudaRedistributablesLoaded misstates what
targetDir points to, so update the documentation to match the real caller flow.
Clarify that targetDir is the stable CUDA redistributable cache directory passed
from InstallOrchestrator (StableCudaRedistDir under
%LOCALAPPDATA%/TheOrc/CudaRedist), not the app’s runtimes/win-x64/native/cuda12
folder, and keep the comment aligned with
NativeBackendBootstrap.PreflightCudaBackend and the StableRedistDir design.
In `@Tools/Get-CudaRedistributables.ps1`:
- Around line 55-57: The success paths in Get-CudaRedistributables.ps1 are
writing the resolved output directory with Write-Host, which keeps it out of the
stdout pipeline. Update the early-exit branch and the later success branch in
the script to emit $OutputDir with Write-Output instead, so callers like the one
using Select-Object -Last 1 can capture the directory reliably.
---
Nitpick comments:
In `@OrchestratorSetup/Services/CudaRedistributableInstaller.cs`:
- Around line 71-79: Dispose the JsonDocument created in
CudaRedistributableInstaller when parsing the manifest, because only RootElement
is kept and the pooled buffers are otherwise leaked. Update the manifest-loading
block in the install flow to keep the JsonDocument alive for the full scope
where manifest is later used in the foreach, using a using declaration or using
statement around JsonDocument.Parse so the document is released after all
manifest access is complete.
In `@Tools/Get-CudaRedistributables.ps1`:
- Line 90: Suppress the PowerShell progress UI around the download in
Get-CudaRedistributables so large archive fetches don’t slow CI; update the
download logic that uses Invoke-WebRequest to temporarily set
$ProgressPreference to SilentlyContinue before the request and restore it
afterward, keeping the behavior localized to this script.
- Around line 89-116: The archive extraction and DLL copy path in
Get-CudaRedistributables.ps1 is missing explicit error handling, so failures
from Expand-Archive or Copy-Item may not surface as a clean controlled exit.
Wrap the extraction and file-copy loop in a try/catch like the existing download
handling, and on any exception Write-Host a clear failure message including the
caught error and exit 1. Keep the fix localized around the Expand-Archive,
Get-ChildItem, and Copy-Item logic so the script always returns a reliable
non-zero signal to callers.
🪄 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: f91d0cad-3dab-4ed1-bce8-e7413d7c27d7
📒 Files selected for processing (10)
.github/workflows/release.ymlOrchestratorIDE/Core/Runtime/AdapterManager.csOrchestratorIDE/Core/Runtime/LLamaSharpRuntime.csOrchestratorIDE/Core/Runtime/NativeBackendBootstrap.csOrchestratorSetup/Services/CudaRedistributableInstaller.csOrchestratorSetup/Services/InstallOrchestrator.csTools/Get-CudaRedistributables.ps1docs/CONTEXT_FABRIC_BENCHMARK_MANIFEST.mddocs/CONTEXT_FABRIC_TEST_HARNESS.mddocs/README.md
- Get-CudaRedistributables.ps1: fix Write-Host -> Write-Output for the resolved directory path -- a real bug, not a nitpick. release.yml captures this via `| Select-Object -Last 1`, and Write-Host writes directly to the console host, bypassing the output stream entirely, so $env:THEORC_CUDA_REDIST_DIR would have been empty and the whole release-build fix silently inert. Verified the fix: the same capture pattern now correctly returns the directory path. - Same script: suppress the default download progress UI ($ProgressPreference = 'SilentlyContinue', large archives on every CI cache-miss) and wrap Expand-Archive/Copy-Item in try/catch for a reliable non-zero exit on failure. - CudaRedistributableInstaller.cs: dispose the JsonDocument instead of only retaining RootElement (was leaking pooled buffers), and correct a stale XML doc comment that described targetDir as the app's own bundle-relative cuda12 folder when it's actually the stable %LOCALAPPDATA% cache directory -- exactly backwards from the design this class exists for. - CONTEXT_FABRIC_TEST_HARNESS.md: fix a leftover "stderr" reference in the diagnostic write-up (the actual code was already fixed to stdout two commits earlier in this same session) and add the actual result from the first real run with the diagnostic enabled: ActiveCount never got stuck, ruling out that hypothesis entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6 tasks
This was referenced Jul 14, 2026
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
Overnight investigation session (2026-07-03/04) into why the CF-7 gate's results couldn't be trusted, and why HARDCOREPC's native runtime broke after installing a CUDA 13.3 Toolkit. Two distinct threads, both landing here after
masterwas reverted back to pre-session state to reopen this as a proper PR for review.Thread 1 — CF-7 test-harness documentation + KV-cache exhaustion bug
docs/CONTEXT_FABRIC_TEST_HARNESS.md: full explanation of how the CF-7 gate builds and grades answers (BuildEvidencePack/BuildTopKText/BuildExhaustiveAnswer,FabricAnswerVerifier, the JSON recovery pipeline), written so the scoring logic can be reviewed independently of any one run's result.NoKvSlotcrashes (KV-cache exhaustion), not real wrong answers —BuildEvidencePack's uncapped evidence packs (up to 26 segments/6.3K tokens per question vs 1-4 before) exhaust the KV-cache pool faster thanAdapterManager's conversation-count-based recycle threshold accounts for.SequenceRecycleThreshold128→24 as a stopgap, validated it empirically, and it made zero difference (byte-for-byte identical failure trace before/after) — documented this honestly rather than let the "fixed it" claim stand.THEORC_KVCACHE_DIAGNOSTICS=1, zero behavior change unless enabled) to actually gather data on the real mechanism, since guessing twice hadn't worked.Thread 2 — Native CUDA backend selection, for dev, CI release, and the installer
Traced via the new diagnostic surfacing (below) that HARDCOREPC's native-library load failures started right after installing a CUDA 13.3 Toolkit:
cuda12backend when its own toolkit-version auto-detection fails (majorCudaVersion == -1). Once a real CUDA Toolkit is present, detection succeeds and returns that exact version, and LLamaSharp then only tries that one folder — nonexistentcuda13in this case — with zero fallback, even though our workingcuda12backend is right there.Cuda12FallbackSelectingPolicy, a small customINativeLibrarySelectingPolicythat forces CUDA candidates back tomajorCudaVersion = -1regardless of detected toolkit — using only public LLamaSharp APIs. Verified live on HARDCOREPC: model loads onto the GPU again (5.7GB VRAM in use), real inference running.FormatLoadFailureonly showed one level ofInnerException(truncating the real cause), andNativeBackendBootstrap.EnsureConfigured()'s returned report was computed and silently discarded at every call site.LLamaSharp.Backend.Cuda12.Windows's NuGet package doesn't ship them, and the CI runner (stockwindows-latest, no GPU, no Toolkit) never had them to bundle. Fixed withTools/Get-CudaRedistributables.ps1, which fetches just the ~3 files needed directly from NVIDIA's own official redistributable manifest feed (SHA-256 verified, no Toolkit install, no third-party NuGet repackaging), wired intorelease.yml.OrchestratorIDE.exepublishes as a self-extracting single-file bundle, so its real runtime directory is a dynamically-generated temp folder the installer can't predict. AddedCudaRedistributableInstallertoOrchestratorSetup, fetching the same redistributables at install time into a stable%LOCALAPPDATA%location (conditional on detected NVIDIA hardware, so other installs don't pay for a download they can't use), plus a smallNativeBackendBootstrapaddition to pre-load from that stable location when the bundle itself doesn't have them.Test plan
Tools/Get-CudaRedistributables.ps1verified end-to-end: manifest fetch, SHA-256 verification, extraction, MSBuild pickupCudaRedistributableInstaller(C# port) verified end-to-end via a standalone harness: fetch, verify, extract, idempotent re-runTHEORC_KVCACHE_DIAGNOSTICS=1to actually root-cause the KV-cache exhaustion (in progress on NEWCOREPC as of this PR)🤖 Generated with Claude Code
Summary by CodeRabbit