Skip to content

fix(local-ai): revert dedicated-memory admission caps for RTX Spark - #1422

Merged
shanselman merged 1 commit into
mainfrom
shanselman-revert-dxgi-admission-cap
Sep 14, 2026
Merged

shanselman merged 1 commit into
mainfrom
shanselman-revert-dxgi-admission-cap

Conversation

@shanselman

Copy link
Copy Markdown
Collaborator

Summary

Surgically revert the dedicated-memory admission cap introduced by 03975b60ac59d96641e46aa3c536e56eea90ee09 in #1253 (fix(local-ai): bound GPU admission by dedicated device memory), including its later NVML fallback. The reported regression affects RTX Spark's 48 GB SKU with a 16 GB carveout: dedicated-memory accounting limits admission to the carveout instead of CUDA-visible memory.

  • Restore cuMemGetInfo total/free memory as the sole GPU-memory authority. Remove DXGI/NVML cap readers and their now-unused LUID interop.
  • Preserve retryable partial hardware facts, device identity, current model selection, and all setup/recovery behavior. Do not revert the whole original PR or later Local AI fixes.
  • Replace cap-specific tests with CUDA-only regressions, including the recorded 46,332 MiB total / 46,114 MiB free Spark shape on x64 and ARM64.

Default setup already skips inference validation. Explicit inference proof and recovery pipelines still run inference and are unchanged. Qualification is not a guarantee that a runtime model load or inference will succeed.

Required proof pools

  • windows-wsl-dgx-blackwell: target RTX Spark unified-memory admission regression. Not verified / blocked: no target Spark hardware available in this session.
  • windows-11-arm64: target ARM64 Spark hardware behavior. Not verified / blocked: architecture-specific qualification is covered by unit tests, not a native ARM64 run.

Validation

Validated commit: 4723adb7.

  • .\build.ps1: passed all five projects and documentation validation.
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: 3,984 passed, 32 skipped, 0 failed.
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: 2,976 passed, 0 failed. Tray settings isolated with OPENCLAW_TRAY_DATA_DIR; project built first to prevent a fresh-worktree no-op.
  • dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --filter 'FullyQualifiedName~LocalAiPortHandoffTests|FullyQualifiedName~SetupPipelineTests|FullyQualifiedName~LocalAiGpuVerificationTests': 54 passed, 0 failed.
  • Focused shared qualification/hardware tests: 37 passed, 0 failed.
  • OPENCLAW_REPO_ROOT set to the isolated worktree for tests.
  • python .\.agents\skills\autoreview\scripts\autoreview --mode local with the explicit rollback scope: Codex review clean, no accepted/actionable findings.

Real behavior proof

Ran dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-build --no-restore --filter 'FullyQualifiedName~LocalInferenceHardwareProbeTests' --logger 'console;verbosity=detailed' with OPENCLAW_RUN_GPU_PROOF=1.

Current-head live output:

NVIDIA GeForce RTX 4080 SUPER: CUDA total=17170956288 free=15792603136
qualification=Unsupported/InsufficientGpuMemory
Passed: 1, Failed: 0

The hardware test compares the production probe's output to the exact native CUDA snapshot it consumed, avoiding a second free-memory reading that could race other workloads. This proves uncapped CUDA reporting on the local discrete GPU, not Spark compatibility or successful inference.

Not verified / blocked: live RTX Spark 48 GB / 16 GB-carveout admission, native ARM64 execution, and target-device inference. No UI controls, MCP contracts, gateway setup steps, or recovery steps changed.

Revert the DXGI cap from 03975b6 and its later NVML fallback, which exclude RTX Spark 48GB devices with a 16GB carveout. Restore CUDA-reported total/free memory while preserving retryable partial facts, current catalog selection, and setup/recovery behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d82b14c5-913a-4f60-a2d9-2a5877593a5a
@clawsweeper

clawsweeper Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@shanselman shanselman added the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Sep 14, 2026
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 14, 2026
@clawsweeper

clawsweeper Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 14, 2026, 3:51 PM ET / 19:51 UTC.

ClawSweeper review

What this changes

The PR restores CUDA-reported memory for Local AI model qualification, removes dedicated-memory caps, and updates hardware tests and setup documentation.

Merge readiness

⛔ Blocked before merge - 4 items remain

The rollback remains necessary relative to current main and contains no definite introduced correctness defect. Target-device validation merits maintainer judgment because earlier Spark evidence includes both allocation failure and successful inference on different builds.

Priority: P2
Reviewed head: 4723adb7bbc656e7c2a55052f5185e01043a54c0
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused rollback with real native-probe evidence and reported validation; target-platform usability remains the material uncertainty.
Proof confidence 🐚 platinum hermit (4/6) Not applicable: The external-contributor gate does not apply to this collaborator-authored PR. Its real RTX 4080 SUPER trace exercises the production CUDA probe and confirms uncapped reporting; Spark admission and native ARM64 remain explicitly blocked and require the separate maintainer evidence decision.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The external-contributor gate does not apply to this collaborator-authored PR. Its real RTX 4080 SUPER trace exercises the production CUDA probe and confirms uncapped reporting; Spark admission and native ARM64 remain explicitly blocked and require the separate maintainer evidence decision.
Evidence reviewed 9 items Repository policy: Read the complete root AGENTS.md and applicable proof-validation skill. No additional ancestor AGENTS.md files were found for the changed paths, and no maintainer-notes directory exists. This read-only review ran no builds, tests, or bundled reviewers; the checkout remained clean.
Introduced behavior and preserved safeguards: The complete pinned base-to-head diff removes DXGI/NVML caps and returns CUDA total/free readings directly. Per-device partial facts, UUID identity, native memory sanity checks, and System32-only CUDA imports remain. No references to the removed probe types remain in source or tests.
Current main and latest release still impose caps: The pinned main implementation retains the dedicated-memory minimum, and v2026.9.3 contains the same capped probe. Neither establishes the requested rollback as already implemented.
Findings None None.
Security None None.

How this fits together

Local AI qualification converts NVIDIA driver readings into a GPU, model, and runtime selection before setup downloads model files. These readings also influence launch readiness and recovery.

flowchart TD
  A[NVIDIA CUDA driver] --> B[Hardware probe]
  B --> C{Hardware facts complete?}
  C -->|No| D[Retryable unavailable state]
  C -->|Yes| E[Model qualification]
  E --> F[Setup and model download]
  F --> G[Runtime launch and recovery]
Loading

Decision needed

Question Recommendation
Should target Spark setup and recovery evidence gate this rollback, or is the documented CUDA-only admission risk acceptable with the current discrete-GPU proof? Require target-device evidence: Retain the rollback and resolve the declared Spark/ARM64 coverage gap before landing, including fresh setup and recovery with the current model catalog.

Why: The intended policy reversal is explicit, but historical Spark outcomes conflict across builds and the current proof never admits a model.

Before merge

  • Resolve merge risk (P1) - Removing both capacity and free-memory bounds affects every CUDA GPU. A newly admitted device may download a model or choose a larger profile that later fails to load; the historical Spark failure and later successful integration used different builds.
  • Resolve merge risk (P1) - Current-head Spark admission, native ARM64 behavior, and fresh-versus-existing-install recovery remain unverified. The RTX 4080 SUPER trace cannot settle those hardware-specific outcomes.
  • Complete next step (P2) - Resolve the Spark proof-sufficiency decision: require current-head target setup/recovery evidence or explicitly accept the documented runtime-allocation risk.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production +15/-522; tests +72/-398; docs +13 The reduction removes the two cap readers and their corresponding tests while retaining the CUDA probe.

Merge-risk options

Maintainer options:

  1. Resolve the Spark coverage gap (recommended)
    Use current-head target-device results to establish that newly admitted models load and existing installations recover successfully.
  2. Accept optimistic qualification
    Explicitly accept possible model-load failures after admission while retaining the existing runtime error and recovery paths.

Technical review

Best possible solution:

Supported Spark installations should qualify and remain usable through fresh setup and recovery, with allocation failures handled by the existing runtime diagnostics.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes that main caps the reported 46,332 MiB CUDA capacity at a 16,320 MiB dedicated reading and excludes catalog models. Successful inference on the target SKU at this head remains unverified.

Is this the best way to solve the issue?

Yes for the explicitly requested admission rollback: the patch removes the cap machinery without undoing identity, retry, catalog, or recovery improvements. Its hardware usability claims still need the stated evidence decision.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against bd9ce43b4d9c.

Labels

Label changes:

  • add P2: This addresses a bounded Local AI hardware-admission problem without evidence of a current product-wide outage.
  • add merge-risk: 🚨 availability: Uncapped capacity and free-memory readings can admit model loads that previously stopped at qualification, with target-device success still unverified.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor gate does not apply to this collaborator-authored PR. Its real RTX 4080 SUPER trace exercises the production CUDA probe and confirms uncapped reporting; Spark admission and native ARM64 remain explicitly blocked and require the separate maintainer evidence decision.

Label justifications:

  • P2: This addresses a bounded Local AI hardware-admission problem without evidence of a current product-wide outage.
  • merge-risk: 🚨 availability: Uncapped capacity and free-memory readings can admit model loads that previously stopped at qualification, with target-device success still unverified.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor gate does not apply to this collaborator-authored PR. Its real RTX 4080 SUPER trace exercises the production CUDA probe and confirms uncapped reporting; Spark admission and native ARM64 remain explicitly blocked and require the separate maintainer evidence decision.

Evidence

What I checked:

  • Repository policy: Read the complete root AGENTS.md and applicable proof-validation skill. No additional ancestor AGENTS.md files were found for the changed paths, and no maintainer-notes directory exists. This read-only review ran no builds, tests, or bundled reviewers; the checkout remained clean. (AGENTS.md:97, 4723adb7bbc6)
  • Introduced behavior and preserved safeguards: The complete pinned base-to-head diff removes DXGI/NVML caps and returns CUDA total/free readings directly. Per-device partial facts, UUID identity, native memory sanity checks, and System32-only CUDA imports remain. No references to the removed probe types remain in source or tests. (src/OpenClaw.Shared/Inference/CudaHostHardwareProbe.cs:145, 4723adb7bbc6)
  • Current main and latest release still impose caps: The pinned main implementation retains the dedicated-memory minimum, and v2026.9.3 contains the same capped probe. Neither establishes the requested rollback as already implemented. (src/OpenClaw.Shared/Inference/CudaHostHardwareProbe.cs:180, 84928c4370bc)
  • Real native probe evidence: The captured PR body reports a current-head RTX 4080 SUPER run: CUDA total 17170956288 bytes, free 15792603136 bytes, and Unsupported/InsufficientGpuMemory. The hardware test delegates to the real native reader and compares the exact consumed snapshot. This demonstrates uncapped reporting, but not Spark admission, ARM64 execution, or successful inference. Captured context sourceRevision: aa4fb3a1c6673140b8a931c799e385210580737ce139fea5b9acbe876560400c. (tests/OpenClaw.Shared.Tests/LocalInferenceHardwareProbeTests.cs:14, 4723adb7bbc6)
  • Setup and recovery boundary: Default setup already omits inference verification; explicit proof and recovery pipelines retain it. Preflight consumes qualification before downloads. The PR changes none of these pipeline steps, so the historical whole-setup rollback must not be attributed to this patch. (src/OpenClaw.SetupEngine/SetupPipeline.cs:54, 4723adb7bbc6)
  • Historical allocation failure: Local AI unusable on DGX Spark (Windows ARM64): shared GPU memory is not CUDA-allocatable, and a capability failure rolls back the whole setup #1237 (Local AI unusable on DGX Spark) records Windows ARM64 driver 592.96 and llama-server b10488 reporting 46,332 MiB CUDA total, then failing model allocation. This closed, unmerged research PR is relevant risk evidence, not a current-head reproduction or replacement candidate.

Likely related people:

  • shanselman: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • joelagnel: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add current-head Spark evidence covering newly eligible model loading and recovery of an existing installation.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@shanselman

Copy link
Copy Markdown
Collaborator Author

Landing decision: proceed with the requested surgical rollback. The maintainer explicitly requested restoring CUDA-only admission for the reported RTX Spark 48 GB / 16 GB-carveout regression and subsequently directed this PR to be merged after the target-hardware limitation had been disclosed. Both the local scoped Codex review and ClawSweeper identify no actionable introduced correctness defect. CI Gate, core/CLI, tray/setup/integration, UI/accessibility, setup/connect and recovery E2E, and CodeQL checks have now passed at 4723adb. We accept the documented qualification tradeoff for this rollback: CUDA-visible capacity can admit a model that later fails to load. This is not a claim of current-head Spark inference or native ARM64 proof; those remain unverified. Default setup no longer gates installation on inference. Recovery and runtime error handling are unchanged. No further scope expansion is warranted for this revert.

@shanselman
shanselman merged commit 6466a04 into main Sep 14, 2026
25 of 26 checks passed
@shanselman
shanselman deleted the shanselman-revert-dxgi-admission-cap branch September 14, 2026 20:00
@shanselman shanselman removed the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant