Add support for ROCm channels (For llama.cpp and stable diffusion)

[superm1]

The ROCm stable runtime utilizes the AMD released ROCm runtimes (stable and preview) and the upstream llama.cpp artifacts.

[jeremyfowers]

Gently blocking PR until policy questions are resolved.

[superm1]

I've made adjustments.

1. We now have 3 ROCm channels.

• stable: latest stable ROCm runtime from AMD (Linux) or latest HIP SDK release from AMD (Windows) + upstream llama.cpp artifacts
• preview: latest preview ROCm runtime from AMD for Windows and Linux + llama.cpp artifacts from https://github.com/lemonade-sdk/llama.cpp
• nightly: nightly ROCm release from AMD combined with latest llama.cpp (from https://github.com/lemonade-sdk/llamacpp-rocm)

2. Stable diffusion is now here too.

This needs more testing that I didn't break anything.

[Geramy]

I've made adjustments.

1. We now have 3 ROCm channels.

• stable: latest stable ROCm runtime from AMD (Linux) or latest HIP SDK release from AMD (Windows) + upstream llama.cpp artifacts
• preview: latest preview ROCm runtime from AMD for Windows and Linux + llama.cpp artifacts from https://github.com/lemonade-sdk/llama.cpp
• nightly: nightly ROCm release from AMD combined with latest llama.cpp (from https://github.com/lemonade-sdk/llamacpp-rocm)

2. Stable diffusion is now here too.

This needs more testing that I didn't break anything.

Perfect, I will try to take the same path for my mlx engine and mlx port then.

[ramkrishna2910]

@claude review

[github-actions]

Claude finished @ramkrishna2910's task in 2m 43s —— View job

---

PR Review: Add support for ROCm channels

Tasks:

• Gather context and understand request
• Read changed files
• Analyze code quality, architecture, and correctness
• Post comprehensive review feedback

---

Overall, this is a substantial and well-structured PR that adds meaningful capability — three ROCm channels (stable/preview/nightly) for both llama.cpp and stable-diffusion.cpp. The core architecture is sound. I've found several bugs and code quality issues below, ranging from config documentation mismatches to hardcoded version strings.

---

Bugs

1. rocm_channel config key location mismatch in docs

defaults.json defines rocm_channel at the top level:

{
  "rocm_channel": "preview",
  "llamacpp": { ... }
}

But docs/llamacpp.md documents it nested under llamacpp:

{
  "llamacpp": {
    "rocm_channel": "preview"
  }
}

The CLI command in the same doc (lemonade config set llamacpp.rocm_channel stable) is also wrong — it should be lemonade config set rocm_channel stable. This will cause user confusion and broken config. Fix this →

2. Windows sd-cpp stable filename has wrong ROCm version

In sd_server.cpp, the stable channel filename hard-codes 7.1.1:

params.filename = "sd-" + short_version + "-bin-win-rocm-7.1.1-x64.zip";

But backend_versions.json defines "rocm-stable-runtime": "v7.2.1". This is either wrong (stale version from before) or intentional (the sd-cpp Windows artifact was built against a different SDK release). Either way it needs a comment explaining the discrepancy, or it should derive the version from backend_versions.json like the Linux path does. Fix this →

3. Incorrect fallback in get_backend_runtime_version for sd-cpp

In backend_manager.cpp, the third fallback in get_backend_runtime_version always falls through to the llamacpp section:

// Falls through to llamacpp section even when recipe == "sd-cpp"
if (backend_versions.contains("llamacpp") &&
    backend_versions["llamacpp"].contains(runtime_key) ...

This means if sd-cpp's runtime key is missing, it would silently use llamacpp's runtime version — wrong and hard to debug. Fix this →

4. ROCm preview filenames hard-code 7.12 instead of reading from backend_versions.json

In llamacpp_server.cpp:

params.filename = "llama-" + version + "-bin-win-rocm-7.12-x64.zip";
params.filename = "llama-" + version + "-bin-ubuntu-rocm-7.12-x64.tar.gz";

The TheRock version is already in backend_versions.json ("therock": { "version": "7.12.0" }). This string should be derived from there rather than hardcoded, otherwise updating the TheRock version requires changes in multiple places. (This was also flagged by @jeremyfowers.) Fix this →

---

Code Quality

5. normalize_backend_name / resolve_*_backend logic duplicated in 5+ places

The pattern if (backend == "rocm") { channel = cfg->rocm_channel(); return "rocm-" + channel; } appears in:

• backend_manager.cpp → normalize_backend_name()
• llamacpp_server.cpp → resolve_llamacpp_backend()
• sd_server.cpp → resolve_sdcpp_backend()
• backend_utils.cpp → inline in get_backend_binary_path() and get_backend_version()
• server.cpp → inline in handle_image_upscale()

This should be a single shared utility — e.g., BackendUtils::resolve_rocm_backend(recipe, backend) in the header. Five copies means a future channel addition requires touching all five.

6. ROCM_STABLE_RUNTIME_DIR constant duplicated in 3 files

static const char* ROCM_STABLE_RUNTIME_DIR = "rocm-stable-runtime";

Defined separately in llamacpp_server.cpp, sd_server.cpp, and backend_manager.cpp. Should be one definition in a shared header (e.g., backend_utils.h).

7. Indentation inconsistency in sd_server.cpp get_install_params

The #elif defined(__linux__) block mixes indentation levels inside if (is_rocm_backend(resolved_backend)):

#elif defined(__linux__)
    if (resolved_backend == "rocm-stable") {        // 4-space indent (correct)
        ...
    } else {
        ...
    }
...
    } else {
        // CPU build (default)
    #ifdef _WIN32                                     // 4-space + `#ifdef` indent (wrong)

The #ifdef _WIN32 in the CPU block is indented with 4 spaces, inconsistent with the project's style where preprocessor directives are at column 0 (or 1 # at position 0).

8. get_current_os() uses #ifdef in anonymous namespace

The new get_current_os() function uses #ifdef _WIN32 / __APPLE__ / __linux__ guards — the reviewer mentioned these were being removed from the codebase. The existing BackendUtils methods already have OS-specific dispatch patterns. It would be more consistent to add get_current_os() as a static method on BackendUtils or SystemInfo if needed, or use the existing __linux__ guard pattern at the call site.

---

Testing Gaps

9. CI test (test-rocm-stable-channel) only verifies config, not inference

The new CI job runs on windows-latest which has no AMD GPU. It validates that:

• rocm_channel can be written to config.json
• recipes output doesn't show rocm-stable/rocm-preview as raw names

But it cannot actually test that the correct binary is downloaded, that the ROCm runtime is installed, or that inference works. The test is worth keeping for the config-layer validation, but the description should reflect what it actually tests.

10. No test for the rocm-nightly channel

The nightly channel has no CI coverage at all. At minimum, validating that validate_backend_choice and normalize_backend_name accept it would help.

---

Minor Notes

• sd_server.cpp: is_rocm_backend() accepts "rocm", "rocm-stable", "rocm-preview" but not "rocm-nightly". If a user manually sets the nightly channel and then sd-cpp resolves to "rocm-nightly", it would fall through to the CPU code path silently rather than erroring.
• install_therock in backend_utils.cpp doesn't pre-delete a stale archive before downloading (unlike install_rocm_stable_runtime_if_needed which does). If a partial previous download exists under the same temp filename, the behavior is undefined.
• get_rocm_stable_runtime_version() and get_therock_version() in sd_server.cpp each independently re-parse backend_versions.json from disk. This is called during get_install_params() — minor, but an easy win to avoid the double parse.