fix: skip optional draft checkpoint in are_required_checkpoints_complete (#2435)#2465
Conversation
…r OpenAI compat Closes lemonade-sdk#1370 — OpenCode / @ai-sdk/openai-compatible streaming crash Three fixes for reasoning model streaming: 1. Streaming proxy normalization (streaming_proxy.cpp): - Intercepts each SSE data: {...} line in forward_sse_stream() - Injects content: "" when reasoning_content is present without content - Injects role: "assistant" when null/missing on assistant deltas - Only applies to chat.completion.chunk objects (non-chat passthrough) 2. Non-streaming response (server.cpp): - Same content injection for REST chat completions response 3. thinking: false passthrough (server.cpp): - Replaced strip_handled_thinking_fields() (which erased enable_thinking/ thinking before forwarding) with normalize_thinking_fields() which renames thinking → enable_thinking and keeps it in the forwarded request. FLM/vLLM/cloud backends now see enable_thinking. - /no_think prefix retained for llama.cpp compatibility Tests: 11 unit tests covering role normalization, reasoning content normalization, carriage return, multi-choice, multi-line streams. 7/7 C++ tests pass (100%).
Closes lemonade-sdk#2371 — multi-GPU systems only got ROCm for the first GPU arch get_rocm_arch() iterates AMD GPUs (iGPU first, then dGPU) and returns only the first match. On systems with both an iGPU and dGPU with different architectures, TheRock was only installed for the iGPU. Fix: - Add get_rocm_arches() returning ALL detected AMD GPU architectures (deduplicated, iGPU-first ordering preserved) - Update install_therock_if_needed() to install TheRock for every arch - Keep get_rocm_arch() for backward compat (rocm_channel, display)
…dk#1364, lemonade-sdk#1546) lemonade-sdk#1364 — Large Prompts Timing Out - Add SSE keepalive heartbeat thread in forward_sse_stream() - Sends : keepalive\n\n every 10s during prefill while waiting for first token - Prevents client-side read timeouts on long-running prompt processing - Thread-safe via shared mutex with the libcurl write callback lemonade-sdk#1546 — Model Download Resilience - Add .completed sentinel written after all files are verified in download - is_checkpoint_path_complete() checks for .completed as authoritative marker - Prevents corrupt partially-downloaded files from appearing complete - Hardened recursive_directory_iterator with skip_permission_denied + error_code
Add a pre-load memory check in Router::load_model() that compares model_info.size (file size in GB) against get_available_memory_gb() for the target device. Logs a warning when the model may not fit. Chose warn-only (not block) because: 1. GGUF file size != load-time memory (mmap'd, paged) 2. Auto-tune ctx_size resolver will reduce context to fit 3. A hard block would frustrate users who know their setup
Completes fixes for lemonade-sdk#1364, lemonade-sdk#1546, lemonade-sdk#1804: ## lemonade-sdk#1364 — SSE heartbeat during long prefill Injects : keepalive\n\n every 10s during prefill to prevent client-side read timeouts on long prompts (15k tokens → 5 min prefill). ## #1546b — .completed sentinel for download verification Written after all files are verified in download_from_huggingface(). is_checkpoint_path_complete() checks for it, preventing corrupt partially- downloaded files from appearing complete after a crash. ## #1546a — Model-level download resume fast-path download_from_huggingface() now accepts do_not_upgrade flag. When set and .completed sentinel exists, skips the HF API call entirely. ## #1546c — Directory iterator hardening discover_extra_models() uses skip_permission_denied + error_code handling to prevent crashes on temp files from interrupted downloads. ## lemonade-sdk#1804 — Pre-load OOM guard Upgraded the pre-load memory check from warning to hard block when model size exceeds 2x available memory headroom. Prevents OOM killer crashes with a clear error message instead. ## CI — PR-Agent + Qodo dual review Added pr-agent-review.yml (DeepSeek) and qodo-merge.yml workflows.
Before fetching the HF API and rebuilding the file list, check for an existing .download_manifest.json with incomplete files. If found, resume downloading from the partial state instead of starting over. This avoids re-downloading already-completed files after a network interruption or Ctrl-C during model pull.
This reverts commit 11991bc.
…ection (lemonade-sdk#2414) The function used a std::lock_guard which caused a deadlock when build_recipes_info re-entered get_system_info_with_cache via get_rocm_arch(). Fix by switching to std::unique_lock, marking s_recipes_computed early, and unlocking during recipe computation. On failure the flag resets so the next call retries.
…ete (lemonade-sdk#2435) The draft checkpoint is optional — the model can run without MTP spec decoding. But are_required_checkpoints_complete() iterates over ALL checkpoint types, and if the draft model hasn't been downloaded yet the entire model gets marked as not-downloaded / 'unreadable'. Fix by skipping the draft type alongside npu_cache.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7048b17f32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| double headroom = available * SAFETY_MARGIN; | ||
| if (model_info.size > headroom * 2.0) { |
There was a problem hiding this comment.
When get_available_memory_gb() cannot determine capacity it returns 0.0 (for example CPU/NPU loads on an Intel/NVIDIA host, where the CPU branch has no total-RAM source), so headroom becomes 0 and any positive-size model hits this new throw. That turns previously loadable CPU models into immediate failures on common systems; treat zero/unknown availability as “cannot preflight” instead of “no memory”.
Useful? React with 👍 / 👎.
| if (heartbeat_thread.joinable()) { | ||
| heartbeat_thread.join(); |
There was a problem hiding this comment.
Because the heartbeat thread is usually blocked inside the 10-second sleep_for() when a fast stream finishes, this join() waits for that sleep to expire before flushing the tail, sending [DONE], and calling sink.done(). In the common case where the first token arrives quickly, every streaming response can hang for up to 10 seconds at the end; use an interruptible wait/condition variable or otherwise wake the thread when stopping it.
Useful? React with 👍 / 👎.
| if (entry.is_directory() && safe_exists(entry.path() / ".completed")) { | ||
| LOG(INFO, "ModelManager") << "Model already downloaded (found .completed sentinel), skipping API call" | ||
| << std::endl; | ||
| return; |
There was a problem hiding this comment.
When do_not_upgrade is true, this returns as soon as any snapshot in the repo has .completed, but that sentinel only proves the files selected for some earlier model/variant were downloaded. The auto-load path calls this after is_model_downloaded() says the requested model is missing, so a second GGUF quant or auxiliary checkpoint in the same repo will skip the HF fetch, be marked downloaded, and proceed with an empty/missing resolved_path; verify the requested checkpoints before returning.
Useful? React with 👍 / 👎.
| // indistinguishable from a complete download by other checks alone. | ||
| // The sentinel is written after all files are verified in | ||
| // download_from_huggingface(). | ||
| return safe_exists(marker_dir / ".completed"); |
There was a problem hiding this comment.
For directory checkpoints this now requires .completed in marker_dir, but existing directory downloads from older Lemonade builds never have that marker, and variant-less vLLM resolves to the cache root while the new downloader writes .completed under snapshots/<sha>. After a cache rebuild/restart those complete models are reported as not downloaded even when there is no manifest or partial file; check the active snapshot marker or preserve the previous complete-directory fallback.
Useful? React with 👍 / 👎.
| types: [opened, synchronize, reopened, ready_for_review] | ||
| jobs: | ||
| qodo-merge: | ||
| if: ${{ secrets.QODO_API_KEY != "" }} |
There was a problem hiding this comment.
GitHub Actions docs state that secrets cannot be directly referenced in if: conditionals, so this job-level expression (and the same pattern in pr-agent-review.yml) will not reliably evaluate as intended when the workflow is parsed. For these optional review jobs, expose a non-secret env/value in a step or move the check into the action step instead of gating the job directly on secrets.*.
Useful? React with 👍 / 👎.
| s_recipes_computed = true; | ||
| lock.unlock(); |
There was a problem hiding this comment.
Setting s_recipes_computed before s_cached_system_info["recipes"] is written lets any concurrent caller during the first recipe build take the early return path with a cache that has no recipes object yet. In that window, calls such as get_supported_backends() or get_flm_status() can report recipes as missing/unsupported and skip model discovery or reject loads, so keep a separate in-progress state or only mark the cache complete after installing the recipes result.
Useful? React with 👍 / 👎.
| bool enable = true; | ||
| if (thinking.is_boolean()) { | ||
| enable = thinking.get<bool>(); | ||
| request_json["enable_thinking"] = enable; |
There was a problem hiding this comment.
should_disable_thinking() explicitly gives enable_thinking precedence, but this normalization overwrites an existing enable_thinking whenever a client also sends thinking. For conflicting requests such as enable_thinking:false with thinking:true, FLM/vLLM/cloud backends will see enable_thinking:true even though the earlier decision disabled thinking, so only map thinking when the canonical field is absent.
Useful? React with 👍 / 👎.
| } | ||
| }); | ||
|
|
||
| auto result = utils::HttpClient::post_stream( |
There was a problem hiding this comment.
If post_stream throws before reaching the cleanup block—e.g. backend connection failure, timeout, or CURLE_WRITE_ERROR after a streaming client disconnects—the local heartbeat_thread is still joinable during stack unwinding, so its destructor calls std::terminate and can crash lemond instead of letting WrappedServer handle the streaming error. Guard this call with RAII/try-catch cleanup so the heartbeat is stopped and joined on every exit path.
Useful? React with 👍 / 👎.
1 participant
Description
Fixes #2435 — adding a `draft` checkpoint to `user_models.json` caused the entire model to become unreadable.
Root Cause
`are_required_checkpoints_complete()` iterates over all checkpoint types, including the optional `draft` MTP checkpoint. If the draft model hasn't been downloaded yet (or has a stale/invalid path), the entire model is marked as not-downloaded — making it invisible in the UI and unusable.
The `draft` checkpoint is purely optional: the model runs without MTP speculative decoding just fine. Only `main` is truly required.
Fix
Skip `draft` checkpoint validation in `are_required_checkpoints_complete()`, the same way `npu_cache` was already skipped.
Testing