feat(local-ai): route RTX Spark unified-memory SKUs through NVIDIA's fixed recipe table - #1520
Conversation
Adds GpuInfo.IsRtxSpark, additive only. Groundwork for routing RTX Spark's unified-memory SKUs through a fixed recipe table instead of the generic capacity fit-test.
RTX Spark's total CUDA-visible memory identifies the physical memory SKU, not usable capacity the way it does on a discrete GPU, so the generic priority/fit-test default pick doesn't apply. Adds: - SpeculativeDecodingMode.None/DraftDFlash and an optional separate draft checkpoint (LocalModelRunRecipe.DraftWeights), needed because DFlash uses an independently pinned draft GGUF rather than an embedded MTP draft layer. - Three RTX Spark catalog recipes: Qwen3.6-35B-A3B IQ4_XS (48GB SKU), and Qwen3.8-27B Q4_K_M with DFlash n=7 (128GB SKU default). The 64GB-SKU recipe reuses the existing default model's ctx-131072-q8_0 profile -- no new catalog entry needed. - RtxSparkInferenceSelector, a fixed SKU-to-recipe table keyed off GpuVisibleMemoryBytes (empirically verified against a real 48GB RTX Spark unit). Wired into LocalInferenceSelector.Select only for the no-requested-model default path; explicit model requests and every non-Spark GPU keep using the untouched generic fit-test. - GetRequiredMemoryBytes/GetDraftKvCacheMemoryBytes now account for a DFlash recipe's separate draft weights and skip draft KV entirely for SpeculativeDecodingMode.None.
BuildPreset hardcoded spec-type = draft-mtp unconditionally. Branches on LocalModelRunRecipe.SpeculativeDecoding so DraftDFlash recipes emit spec-type = draft-dflash plus spec-draft-model, and None emits no spec-* lines at all. DraftDFlash still requires a resolved draftModelPath; that path isn't threaded from the install manifest yet (the DFlash draft checkpoint isn't acquired/verified through HuggingFaceModelInstaller in this change), so BuildPreset fails closed with InvalidDataException rather than silently omitting the draft model. Follow-up work.
Adds SKU-boundary coverage for the new RtxSparkInferenceSelector (32/48/64/128GB) and a case proving a non-Spark GPU with Spark-sized memory still takes the generic path. Evaluate_RoutesRuntimeByArchitectureWithoutGpuSkuPairing used "NVIDIA RTX Spark N1X" as an arbitrary placeholder name to prove GPU name doesn't affect runtime routing. That's no longer SKU-irrelevant now that RTX Spark has real SKU routing, so the fixture GPU name is swapped to a generic dGPU.
LocalModelCatalog.AdditionalArtifacts() gives every acquirer, manifest, and launch path a single fixed ordering for a recipe's non-primary pinned artifacts. Today that is the DFlash draft checkpoint. GetRequiredMemoryBytes now includes the draft checkpoint's weights, so a DFlash recipe does not under-report the memory it needs during qualification.
b10655 (CUDA 13.3 on x64) predates DFlash draft-decoding support and the 96GB recipe's validated build. Bumping to b11026 puts both x64 and arm64 on CUDA 13.4 and covers every RTX Spark recipe with one pinned runtime, matching the existing single-global-pin design instead of adding per-recipe runtime routing.
…sets Extends LocalAiInstallManifest with AdditionalModelAssets/ AdditionalModelPaths (schema 5) so a recipe's DFlash draft checkpoint or extra split-GGUF shards can be recorded and re-verified alongside the primary weights receipt in the same Hugging Face hub cache. Schema 4 manifests are untouched -- the new fields are empty and absent from JSON unless a recipe actually pins additional assets. Each additional-asset receipt derives its own repository/revision from its own SourceUrl (not the primary ModelId) since the DFlash draft checkpoint is pinned from a different HF repo than the primary weights. Adds LocalAiInstallManifest.UsesHubCache so schema 4 and schema 5 are treated identically everywhere the manifest previously branched only on the exact HubCacheReceiptSchemaVersion value. AdditionalModelAssets/AdditionalModelPaths are left at their unset ImmutableArray default (not .Empty) so JsonIgnoreCondition.WhenWritingDefault actually omits them for schema-3/4 manifests -- .Empty is a distinct, non-default array instance the condition never matches, so writing it would have added new fields to every existing schema-4 receipt and broken older app builds' strict unknown-field rejection. UsesHubCache is marked [JsonIgnore] for the same reason: it's a derived read helper, not part of the persisted contract. ResolveAndValidate normalizes the unset default to .Empty immediately after load so every in-memory reader keeps using ordinary IsEmpty/Length calls safely.
Adds HuggingFaceModelInstaller.InstallAdditionalAssetAsync, mirroring InstallAsync's resumable-download/verify/promote flow for a recipe's non-primary pinned artifacts (DFlash draft checkpoint, extra split-GGUF shards). Deliberately duplicated rather than refactored out of InstallAsync to avoid any risk to that heavily-tested primary weights path; the one thing it omits is the legacy app-owned compatibility copy, since every recipe using an additional asset is new since the hub cache became the primary store.
…ncile AcquireLocalAiModelStep now downloads/verifies a recipe's additional assets (via LocalModelCatalog.AdditionalArtifacts) right after its primary weights, records the results on SetupContext, and rolls the context field back on failure -- the hub-cache artifacts themselves survive rollback the same way the primary weights' do, since there is no legacy copy to delete. PersistLocalAiManifestStep writes them into a schema-5 manifest (schema 4 unchanged for recipes with none), and leaves the two new manifest fields at their unset default rather than an explicitly-built empty array when there is nothing to add, so a plain schema-4 install keeps omitting them from JSON. LocalAiInstallReconciler gains VerifyAdditionalAssetAsync so a reused install re-verifies a recipe's additional assets against the hub cache, not just its primary weights, before treating the install as still valid. LocalAiReconcileResult now also carries the additional asset installs it just verified, reconstructed from the manifest's own already-verified receipts: recovery for a broken runtime (model and additional assets still valid) previously left SetupContext's additional-install list empty because AcquireLocalAiModelStep's reuse skip never re-runs acquisition, which made PersistLocalAiManifestStep hard-fail on the very installs this series adds an acquisition step for. The setup review consent screen now also lists every additional artifact (a DFlash draft checkpoint, or extra split-GGUF shards) as its own download line instead of only the primary weights.
…unch preset BuildCore resolves the DFlash draft checkpoint and passes it to BuildPreset, so spec-draft-model is finally emitted for real. ValidateArtifactReceipts also checks additional-asset receipts against the catalog, as it already does for the primary weights and runtime artifacts. The path handed to llama-server is the handle-resolved physical path from the same verification that opened the file, not the persisted snapshot path. The hub cache hands out a handle-resolved path precisely so a snapshot-link replacement cannot change the file identity a native reader finally opens, which is how the primary model is already bound; the draft checkpoint now gets the same guarantee instead of re-deriving its path from its receipt. LlamaServerRuntimeService's schema checks use LocalAiInstallManifest.UsesHubCache so schema-5 installs are treated as hub-cache-backed, matching schema 4, instead of falling into the legacy schema-3 branch.
A recipe's pinned weights are not everything it downloads or loads: a DFlash recipe also pulls a separate draft checkpoint, and llama-server loads both. Ranking, the post-launch GPU-load sanity check, and the user-facing size disclosure all read the primary weights alone, so a DFlash recipe understated its footprint and the setup review omitted the draft checkpoint entirely. Adds LocalModelCatalog.TotalDownloadSizeBytes (weights plus draft checkpoint) and switches SelectDefaultModelAndProfile's tie-break and fallback, LocalAiGpuVerification's minimum-load-delta check, and the setup UI's model picker and detail text to use it. SelectDefaultModelAndProfile also excludes priority-0, explicit-alternative models from the generic default and fallback pool, so a model reachable only through a specific SKU can never win the generic pick as the catalog grows.
…are incomplete An RTX Spark GPU whose CUDA memory couldn't be read reads as 0 bytes, which the fixed SKU table treated as a legitimately-too-small SKU (NotRecommendedForSku) instead of the real problem: unreadable capacity. Select() now only takes the Spark SKU-routing branch when the Spark GPU's facts are actually complete, so an incomplete-facts Spark GPU falls through to the generic path and gets correctly diagnosed as HardwareFactsIncomplete, same as any other GPU. Also bumps LocalAiPortHandoffTests' Spark fixture from an arbitrary ~24GiB (a pre-SKU-routing placeholder, now below the smallest 32GB tier) to a real 48GB SKU's measured cuMemGetInfo total, so it again resolves to the 24GB recipe instead of "not recommended."
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
karkarl
added
the
status: 🚢 actively landing
clawsweeper
Bot
added
P2
|
Codex review: blocked before merge. Reviewed September 29, 2026, 1:39 PM ET / 17:39 UTC (Revision 10). ClawSweeper reviewWhat this changesThe branch selects Local AI recipes by RTX Spark memory SKU, adds verified draft-model downloads and schema-5 install receipts, upgrades llama-server, and updates setup, tray, and upgrade rollback behavior. Merge readiness⛔ Blocked before merge - 6 items remain Current main and the latest release still use generic Local AI selection, so this PR remains useful. The earlier runtime-upgrade review blockers are addressed, but two 32 GB Spark UI and recovery defects remain. The 32 GB default policy and current-head native upgrade proof also need maintainer resolution. Priority: P2 Review scores
Verification
How this fits togetherLocal AI takes CUDA hardware facts and a model choice through recipe selection and setup. Setup records verified downloads for the native inference server, while the tray presents its status and recovery actions. flowchart LR
A[CUDA hardware facts] --> B[Recipe selection]
B --> C[Setup availability]
C --> D[Verified downloads]
D --> E[Install receipt]
E --> F[Native inference server]
F --> G[Tray status and recovery]
Decision needed
Why: The linked feature discussion explicitly leaves adoption of this externally supplied SKU rule to a maintainer; source correctness cannot settle the default-product choice. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Give the 32 GB outcome specific localized copy and an in-product Fix or Retry Setup path for stale receipts, then validate a current-head native upgrade before rolling out the approved SKU policy. Do we have a high-confidence way to reproduce the issue? Yes, from source: a 32 GB Spark produces NotRecommendedForSku, which has no dedicated UI reason, and a stale saved model fails the availability gate that controls recovery actions. No live 32 GB UI run was supplied. Is this the best way to solve the issue? Partly. Fixed-SKU routing and the revised rollback path fit the request, but the 32 GB explanation and stale-receipt recovery need repair before this is a complete user flow. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against fdca1f1f80c2. LabelsLabel changes: No label changes. Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (9 earlier review cycles; latest 8 shown)
|
karkarl
added
status: 🚢 actively landing
clawsweeper
Bot
added
merge-risk: 🚨 availability
CI rerun triage at
|
karkarl
removed
the
status: 🚢 actively landing
clawsweeper
Bot
added
rating: 🦪 silver shellfish
shanselman
added
the
status: 🚢 actively landing
shanselman
left a comment
shanselman
left a comment
There was a problem hiding this comment.
Reviewed exact head 7e519933a3444ee0bfa45cffb723568b1c0f45c1 against current main 015897024b28cd1b5bc36c61a0f669d537d74d65. The RTX Spark selection and the first-navigation delayed receipt refresh look correct, but two runtime-upgrade defects block landing:
-
LlamaRuntimeInstaller.Component(LlamaRuntimeVariant runtime)uses the current globalLlamaRuntimeCatalog.ReleaseTaginstead ofruntime.ReleaseTag.LocalAiInstallReconciler.ValidateRecipeMatchcalls it for the installed retired b10655 variant, so a real shipped path underengines\llama-server\b10655\...is validated againstb11026and setup terminates before it can reportruntimeUpgradePending. The newReconciler_UpgradesRetiredRuntimeReceiptInsteadOfFailingSetupfixture masks this because it constructs the retired path with the same buggy helper. Please makeComponentuseruntime.ReleaseTag, and construct the regression fixture from the literal shipped b10655 path so it cannot agree with a broken helper. -
After that is fixed, the
runtimeUpgradePendingearly return inLocalAiInstallReconciler.ReconcileAsyncoccurs beforeMigrateLegacyModelAsync. A schema-3 install is returned as a reused model withCacheRoot: null;AcquireLocalAiModelStepskips it, thenPersistLocalAiManifestSteprejects it because persistence requires a populated hub cache root. Please migrate a verified schema-3 model before returning the runtime-upgrade result, and add an end-to-end setup-step regression proving a schema-3 b10655 receipt upgrades to b11026 and schema 4/5 without downloading the primary model again.
The requested receipt race is covered: ApplyRuntimeSnapshot recomputes availability when the saved model id arrives, and Spark32Gb_WhenReceiptArrivesAfterAvailability_RecomputesAndKeepsRetrySetupReachable is a genuine gated delayed-refresh regression.
Local closeout passed after one unrelated MCP disposal flake passed on immediate rerun: full build, Shared 4,127 passed / 35 skipped, Tray 3,119 passed, focused Connection Local AI 110 passed / 5 skipped, focused SetupEngine Local AI 118 passed / 1 skipped. Exact merge-tree and diff --check against current main are clean. GitHub's Local AI-bearing suites are green, but the required gate remains red from unrelated Setup/connect and Network recovery E2E failures, so I did not bypass or merge.
shanselman
removed
the
status: 🚢 actively landing
karkarl
added
the
status: 🚢 actively landing
clawsweeper
Bot
added
rating: 🧂 unranked krab
Validate shipped runtime paths against their own release, migrate verified legacy models before upgrade reuse, and restore the pre-upgrade receipt on rollback independently of gateway recovery. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e59ac0b-0ccb-4614-bb44-088bb882d0ab
|
@bkudiess @shanselman Addressed the three corroborated runtime-upgrade blockers in 7d25102:
Validation on the pushed head: full build passed; Shared 4,129 passed / 33 skipped; Tray 3,119 passed; focused SetupEngine Local AI 124 passed / 1 skipped; focused Connection Local AI 114 passed / 1 skipped. Rubber-duck review confirmed these three blockers fixed. Structured autoreview could not run the Codex engine because authentication returned HTTP 401; that is explicitly recorded, not claimed clean. The PR body now distinguishes historical hardware evidence from current file-backed pipeline regressions. No new physical GPU upgrade/inference proof or UI capture is claimed. Current-head CI remains a separate gate. Both linked verdicts were summary comments with no inline review threads, so this reply addresses them together without dismissing the changes-requested review. |
karkarl
removed
status: ⏳ waiting on author
clawsweeper
Bot
added
rating: 🦐 gold shrimp
shanselman
added
the
status: 🚢 actively landing
shanselman
left a comment
shanselman
left a comment
There was a problem hiding this comment.
Follow-up: source blockers resolved; one physical upgrade proof remains
Reviewed exact head 7d25102ba653fd04af7356ea02c54f007e36ae2c, limited to the repair delta from 7e519933 and its upgrade, persistence, rollback, and cancellation callers. All three previously reported source blockers are resolved. No further source change is requested.
- Runtime identity now uses the selected variant's release tag. The regression independently uses the literal shipped
engines\llama-server\b10655\win-x64\llama-server.exepath. - Verified schema-3 models migrate to the hub cache before upgrade reuse. The complete reconcile/acquire/persist regression rejects primary-model HTTP and persists b11026/schema 4.
- Normal upgrades preserve a separate receipt baseline. The six schema-3/schema-4 success and failure cases pass, including byte-equal restoration of their original serialized receipts, retained old runtime/model/cache, removal of newly acquired runtime, and retry reconciliation. Gateway recovery guards remain separate. The existing delayed first-navigation receipt regression also passes.
Validation
Native Windows ARM64, isolated tray state, OPENCLAW_REPO_ROOT set to this worktree. Missing fresh-worktree test assets were restored, then all required commands were rerun successfully.
| Command | Result |
|---|---|
.\build.ps1 |
All projects and documentation validation passed |
dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore |
4,127 passed, 35 skipped |
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore |
3,119 passed |
dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore --filter 'FullyQualifiedName~LocalAi' |
124 passed, 1 cross-volume case skipped |
dotnet test .\tests\OpenClaw.Connection.Tests\OpenClaw.Connection.Tests.csproj --no-restore --filter 'FullyQualifiedName~LocalAi' |
110 passed, 5 symlink/cross-volume cases skipped |
The exact-head CI run, including CI Gate, is green. Merge-tree against current origin/main (fdca1f1f) and repair diff --check are clean.
Remaining proof-only gate
@karkarl, one bounded windows-wsl-dgx-blackwell upgrade trace is still needed on qualified NVIDIA hardware; @joelagnel's existing Spark or RTX 5090 host would also suffice:
- Start from a genuine released b10655 schema-3 installation with its real versioned executable path and an unchanged supported model recipe, in isolated/restorable state. Do not just rewrite a b11026 receipt's version fields.
- Run the existing setup upgrade at the reviewed head. Show migration/model reuse without primary-model downloading, the resulting b11026/schema-4 receipt, preservation of install metadata, and retention of the old runtime and model.
- Launch the real b11026 runtime, confirm its healthy endpoint, restart it, and complete the existing fixed non-sensitive inference proof. Publish only redacted step outcomes, runtime/model identifiers, download counts, and receipt comparisons, not raw receipts, credentials, or user paths.
The file-backed pipeline regressions are strong evidence for migration and rollback, but their fake runtime inspector and archive responses do not establish this physical upgrade-to-launch handoff.
Existing evidence is retained where applicable, not discarded because the SHA changed. Compared with hardware-proof head e124cf9b, the complete OpenClaw.Connection\LocalAi runtime/manifest implementation, runtime/model catalogs, Spark and generic selectors, hardware-info code, Hugging Face installers, and setup review summary are byte-identical. The historical native launch/inference and draft-identity proof remains applicable to those unchanged paths. Its seeded upgrade did not establish the genuine shipped old path or the new schema-3 migration.
The 48 GB setup screenshot remains historical evidence for that unchanged 48 GB recommendation/profile/disclosure, not proof of every current UI state. The later configured-32 GB availability fix is outside that capture and remains covered by its existing gated regression. This repair changes no UI presentation: no new UI capture, all-SKU hardware matrix, physical rollback-fault matrix, or MXC/BaseContainer run is requested.
Leaving this unmerged solely for that physical upgrade proof. This supersedes the old source-blocking review; once the trace is supplied, the next review can stay limited to that evidence and any subsequent delta.
The source blockers in this old-head review are resolved at 7d25102. Superseded by current-head proof-only review #1520 (review); no source revisions requested.
shanselman
added
status: 📣 needs proof
Scott explicitly directed normal merge after the remaining physical upgrade/inference proof gap was explained. Source blockers are resolved. This retires the proof-only hold, not the factual limitation: current-head physical b10655/schema-3 upgrade and inference remain Not verified.
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: feat(local-ai): route RTX Spark unified-memory SKUs through NVIDIA's fixed recipe table This is item 1/1 in the current shard. Shard 0/1. This temporary status tracks the active review worker. The completed review will appear in the durable ClawSweeper review comment. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
shanselman
left a comment
shanselman
left a comment
There was a problem hiding this comment.
Maintainer decision for exact reviewed head 7d25102ba653fd04af7356ea02c54f007e36ae2c: Scott explicitly directed "lets just merge" after the sole remaining physical upgrade/inference evidence gap was explained. Accepting that known limitation for landing, without claiming new hardware proof or bypassing repository protection.
All three former source blockers are resolved. The requested independent GPT-6 Astra and Claude Opus 5.5 reviews found no source blocker. Required full build, Shared/Tray tests, focused SetupEngine/Connection Local AI suites, and exact-head required CI Gate passed. No source changes or new review/test cycles were made after that validation.
Not verified: a current-head physical upgrade from a genuine shipped b10655/schema-3 installation through migration, real b11026 launch/restart, and inference. The PR body and prior detailed proof handoff remain factual records of this limitation. Historical evidence is retained only for the unchanged GPU/runtime/UI paths identified there.
Landing by the repository-supported rebase method to preserve Joel Fernandes and Karen Lai as authors of their commits.
shanselman
removed
status: ⏳ waiting on author
4 participants
Closes #1496
Supersedes #1497. This maintainer replacement preserves the contributor's 12-commit RTX Spark change stack without force-pushing the external contributor branch. The original fork branch is contributor-owned and did not expose push permission to this maintainer session.
The original replacement head
7e519933was rebased onto5a595352, including #1515 (fix(setup): retry guarded restart after reload owner handoff); its 12 feature commits were patch-equivalent to original head24ff075a. Current head7d25102ba653fd04af7356ea02c54f007e36ae2cadds the runtime-upgrade review fixes below, so the complete current branch is no longer patch-equivalent to the original proof heads.What changed
The normal-upgrade rollback baseline is now separate from gateway recovery state. Existing gateway recovery endpoint-health/provider rollback guards remain unchanged. Reconciliation also restores the baseline if setup fails after model migration but before manifest persistence. Superseded runtimes are deliberately retained until uninstall.
Required proof pools
windows-wsl-dgx-blackwell: applicable. Original RTX Spark and RTX 5090 evidence is linked below. No physical current-head rerun was collected for the review fixes; synthetic files and fake runtime inspection are not hardware proof.windows-winui-interactive: applicable to the original feature. Original live WinUI evidence is linked below; no current-head visual recapture was collected. The review fixes do not change UI presentation.Validation
Review-fix head
7d25102ba653fd04af7356ea02c54f007e36ae2c, local native Windows ARM64 host:.\build.ps1: passed, all projects built, documentation validation passed.dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: 4,129 passed, 33 skipped, 0 failed.dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: 3,119 passed, 0 failed.OPENCLAW_TRAY_DATA_DIRisolated.dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore --filter 'FullyQualifiedName~LocalAi': 124 passed, 1 cross-volume test skipped, 0 failed.dotnet test .\tests\OpenClaw.Connection.Tests\OpenClaw.Connection.Tests.csproj --filter 'FullyQualifiedName~LocalAi': 114 passed, 1 cross-volume test skipped, 0 failed. First run included restore.git diff --check HEAD^ HEAD: passed.OPENCLAW_REPO_ROOTpointed at this worktree. Shared and Tray were restored and run first, then the required--no-restorecommands were rerun with actual test counts.python .agents\skills\autoreview\scripts\autoreview --mode commit --commit HEAD --stream-engine-outputformed the complete commit bundle but the Codex engine failed with HTTP 401 Unauthorized. No clean structured-review result is claimed. An earlier local-mode attempt was rejected because sanitized Git treated checkout line endings as a 4 MB diff; commit mode avoided that noise without truncating the actual patch.Real behavior proof
Current-head automated upgrade regression uses real temporary files, SHA-256 verification, hub-cache migration, runtime archive extraction and manifest writes through
SetupPipeline; HTTP archive responses and runtime inspection are controlled test doubles. It does not launch a real llama-server process.engines\llama-server\b10655\win-x64\llama-server.exe, independent of the installer helper.Historical physical and interactive evidence for the original feature stack (not evidence for the new upgrade fixes):
Not verified / blocked