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 25, 2026, 7:48 PM ET / 23:48 UTC (Revision 3). ClawSweeper reviewWhat this changesThe branch selects fixed Local AI recipes for RTX Spark memory tiers, downloads and verifies DFlash draft assets, records them in installation receipts, updates setup and tray behavior, and upgrades the managed llama-server runtime. Merge readiness⛔ Blocked before merge - 5 items remain This PR still adds RTX Spark behavior absent from current main. Real-device evidence supports the 48 GB and x64 paths, but a fresh 32 GB Spark receives an inaccurate unavailable message. The 64/128 GB rollout and older-build recovery from schema 5 also need an explicit landing decision. Priority: P2 Review scores
Verification
How this fits togetherLocal AI setup reads GPU facts and selects a pinned model recipe before downloading assets and starting a local inference server. The installation receipt and server state then feed setup and tray availability displays. flowchart LR
A[GPU and memory facts] --> B[SKU or generic selection]
B --> C[Pinned model recipe]
C --> D[Download and verify assets]
D --> E[Installation receipt]
E --> F[Local inference server]
B --> G[Setup and tray availability]
F --> G
Decision needed
Why: Physical proof covers 48 GB Spark and x64 discrete hardware, while the new receipt format is unreadable by older builds; selecting a rollout and rollback contract requires product ownership. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the approved 32 GB no-default policy, display its real reason in setup and tray, and stage higher-tier defaults until native proof and a clear in-product schema-5 recovery path are ready. Do we have a high-confidence way to reproduce the issue? Yes, the fresh lone-32 GB Spark path is clear in source: selection returns NotRecommendedForSku, diagnostics map it to Unknown, and a tray test asserts that result. This read-only review did not execute the app. Is this the best way to solve the issue? Not yet. The fixed recipe and verified asset paths are well supported for the exercised hardware, but the 32 GB message needs a dedicated reason and the higher-tier rollout and rollback contract need owner approval. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5a59535216ee. 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
History |
karkarl
added
status: 🚢 actively landing
clawsweeper
Bot
added
merge-risk: 🚨 availability
Collaborator
Author
CI rerun triage at
|
karkarl
removed
the
status: 🚢 actively landing
This branch has not been deployed
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
2 participants
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.
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 replacement is rebased onto
origin/mainat5a595352, which includes PR #1515 commit7d92747e.git range-diffreports all 12 original commits as patch-equivalent (=) from original head24ff075ato replacement head7e519933.What changed
Required proof pools
windows-wsl-dgx-blackwell: applicable. The original patch-equivalent stack has physical RTX Spark and RTX 5090 proof linked below. Current replacement-head physical rerun is blocked because Crabbox is not installed on the maintainer host. Mocked detection is not being presented as hardware proof.windows-winui-interactive: applicable. The original patch-equivalent stack has live WinUI evidence linked below. Current replacement-head interactive recapture is blocked on this non-DGX maintainer host.Validation
Current replacement head
7e519933:./build.ps1: passed, all projects built.dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore: 4,130 passed, 0 failed, 32 skipped.dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore: 3,119 passed, 0 failed, 0 skipped.OPENCLAW_TRAY_DATA_DIRwas isolated.LocalInferenceQualificationTests: 59 passed, 0 failed. Covers CUDA GPU facts, RTX Spark detection, fixed recipe routing, mixed hosts, and generic GPU fallback.LocalAiInstallRecoveryTests|LocalAiPortHandoffTests: 51 passed, 0 failed, 1 cross-volume test skipped. Covers setup, repair, runtime upgrade, additional assets, and port handoff.LocalAiManifestMigrationTests|LocalAiPortLifecycleTests: 113 passed, 0 failed, 1 cross-volume test skipped. Covers schema migration, verified runtime launch, restart, and endpoint lifecycle.git diff --check origin/main...HEAD: passed.LocalInferenceSelector.csin both old and new diff content. No scanner bypass or truncated review was used.Real behavior proof
This replacement contains no patch changes relative to #1497. The rebase-only equivalence is proven by
git range-diff, with all 12 commits matched as=.Physical and interactive evidence for the patch-equivalent original stack:
Not verified / blocked
A physical DGX/RTX Spark rerun at replacement head
7e519933was not available from this session because no Crabbox executable or approved static DGX host is configured. The linked hardware evidence is from patch-equivalent pre-rebase heads, not claimed as a new current-head hardware run. GitHub CI on the replacement PR remains the exact-head remote validation source.