fix: preserve prior saved recipe options on partial save

A /load with save_options=true persisted only the current request's
recipe-option fields, clobbering fields saved by an earlier request on
the same model — e.g. saving llamacpp_args dropped a previously-saved
ctx_size, and a later option-free load no longer recalled it.

Merge the request over the model's prior saved Layer-3 options
(get_saved_model_options) so earlier user saves survive, without baking
model-JSON / image defaults (Layers 1-2) into recipe_options.json where
they would shadow future registry default changes. Also clears a model's
saved options on delete so a re-pull of the same id starts clean.

Adds a pure-API regression test (test_012p) covering the two-save
clobber sequence and the option-free recall that follows.

Author: ianbmacdonald

src/cpp/include/lemon/model_manager.h

@@ -226,6 +226,11 @@ class ModelManager {
     // removed in the directory.
     void set_extra_models_dir(const std::string& dir);
 
+    // Return ONLY this model's persisted Layer-3 saved options (recipe_options.json),
+    // not the fully-resolved stack — for merge-preserving saves that do not bake defaults.
+    RecipeOptions get_saved_model_options(const ModelInfo& info,
+                                          bool refresh_saved_from_disk = false);
+
     void save_model_options(const ModelInfo& info);
 
     void start_directory_watcher();
@@ -304,6 +309,8 @@ class ModelManager {
 
     // Discover GGUF models from extra_models_dir
     std::map<std::string, ModelInfo> discover_extra_models() const;
+    json get_saved_recipe_options_snapshot(bool refresh_saved_from_disk);
+    void delete_saved_model_options(const std::string& model_name);
 
     json server_models_;
     json user_models_;

src/cpp/server/model_manager.cpp

@@ -660,6 +660,32 @@ static RecipeOptions build_recipe_options(const ModelInfo& info,
     return RecipeOptions(info.recipe, base_options);
 }
 
+static const json* find_model_json_entry(const json& server_models,
+                                         const json& user_models,
+                                         const std::string& model_name) {
+    std::string json_key = model_name;
+    bool is_user_model = is_user_model_name(model_name);
+    if (is_user_model) {
+        json_key = strip_user_model_prefix(model_name);
+    }
+
+    if (is_user_model && user_models.contains(json_key)) {
+        return &user_models.at(json_key);
+    }
+    if (!is_user_model && server_models.contains(json_key)) {
+        return &server_models.at(json_key);
+    }
+
+    return nullptr;
+}
+
+static json extract_json_recipe_options(const json* model_json) {
+    if (model_json && model_json->contains("recipe_options") && (*model_json)["recipe_options"].is_object()) {
+        return (*model_json)["recipe_options"];
+    }
+    return json(nullptr);
+}
+
 // Clean up orphaned HF cache blobs after deleting a symlink.
 // HF hub downloads use: snapshots/<hash>/file.gguf -> ../../blobs/<sha256>
 // If no remaining symlink in the repo points to the blob, it's safe to remove.
@@ -1172,6 +1198,34 @@ std::string ModelManager::get_hf_cache_dir() const {
     return lemon::utils::get_hf_cache_dir();
 }
 
+json ModelManager::get_saved_recipe_options_snapshot(bool refresh_saved_from_disk) {
+    // The save path reads the prior persisted options into a LOCAL (refresh=true)
+    // so the merge sees the authoritative on-disk state without touching the shared
+    // recipe_options_ member — which is read by build_cache()/add_model_to_cache()
+    // under models_cache_mutex_, so reading it here (a different, concurrent path)
+    // would be a data race on the json (invariant #8). A fresh disk parse avoids
+    // that. With refresh=false the caller accepts the in-memory member by value.
+    if (refresh_saved_from_disk) {
+        return load_optional_json(get_recipe_options_file());
+    }
+    return recipe_options_;
+}
+
+RecipeOptions ModelManager::get_saved_model_options(const ModelInfo& info,
+                                                    bool refresh_saved_from_disk) {
+    // Return ONLY this model's persisted Layer-3 options (recipe_options.json),
+    // canonical-keyed — NOT the fully-resolved effective stack. Used by the save
+    // path so a save merges request-over-prior-saved and preserves earlier user
+    // saves WITHOUT baking image_defaults / model-JSON defaults (Layers 1-2),
+    // which would shadow future registry default changes.
+    json snapshot = get_saved_recipe_options_snapshot(refresh_saved_from_disk);
+    const std::string key = cache_key_to_canonical_id(info.model_name);
+    json saved = (snapshot.contains(key) && snapshot[key].is_object())
+                     ? snapshot[key]
+                     : json::object();
+    return RecipeOptions(info.recipe, saved);
+}
+
 void ModelManager::invalidate_models_cache() {
     std::lock_guard<std::mutex> lock(models_cache_mutex_);
     cache_valid_ = false;
@@ -1851,12 +1905,25 @@ void ModelManager::save_user_models(const json& user_models) {
 void ModelManager::save_model_options(const ModelInfo& info) {
     LOG(INFO, "ModelManager") << "Saving options for model: " << info.model_name << std::endl;
     // Persist under canonical ID (built-ins are keyed bare in cache but
-    // recipe_options.json stores them as builtin.<name>).
+    // recipe_options.json stores them as builtin.<name>). Matches upstream's
+    // existing save discipline (recipe_options_ written here, read by the
+    // cache-build paths under models_cache_mutex_).
     recipe_options_[cache_key_to_canonical_id(info.model_name)] = info.recipe_options.to_json();
     update_model_options_in_cache(info);
     save_user_json(get_recipe_options_file(), recipe_options_);
 }
 
+void ModelManager::delete_saved_model_options(const std::string& model_name) {
+    // recipe_options.json is keyed by canonical ID (built-ins as builtin.<name>),
+    // so erase under the canonical key or built-in entries would never be removed.
+    // Same write discipline as save_model_options() above.
+    const std::string key = cache_key_to_canonical_id(model_name);
+    if (recipe_options_.erase(key) > 0) {
+        save_user_json(get_recipe_options_file(), recipe_options_);
+        LOG(INFO, "ModelManager") << "✓ Removed saved recipe options for " << model_name << std::endl;
+    }
+}
+
 std::map<std::string, ModelInfo> ModelManager::get_supported_models() {
     // Build cache if needed (lazy initialization)
     build_cache();
@@ -1874,7 +1941,7 @@ std::map<std::string, ModelInfo> ModelManager::get_supported_models() {
     return public_models;
 }
 
-static void load_checkpoints(ModelInfo& info, json& model_json) {
+static void load_checkpoints(ModelInfo& info, const json& model_json) {
     if (model_json.contains("checkpoints") && model_json["checkpoints"].is_object()) {
         for (auto& [key, value] : model_json["checkpoints"].items()) {
             info.checkpoints[key] = value.get<std::string>();
@@ -2007,7 +2074,6 @@ void ModelManager::build_cache() {
 
     models_cache_.clear();
     std::map<std::string, ModelInfo> all_models;
-    std::map<std::string, json> json_recipe_options;  // Per-model recipe_options from JSON
 
     // Step 1: Load ALL models from JSON (server models)
     for (auto& [key, value] : server_models_.items()) {
@@ -2046,11 +2112,6 @@ void ModelManager::build_cache() {
 
         parse_image_defaults(info, value);
 
-        // Parse recipe_options if present (for per-model runtime config like sdcpp_args)
-        if (value.contains("recipe_options") && value["recipe_options"].is_object()) {
-            json_recipe_options[key] = value["recipe_options"];
-        }
-
         // Populate type and device fields (multi-model support)
         info.type = get_model_type_from_labels(info.labels);
         info.device = get_device_type_from_recipe(info.recipe);
@@ -2099,11 +2160,6 @@ void ModelManager::build_cache() {
 
         parse_image_defaults(info, value);
 
-        // Parse recipe_options if present (for per-model runtime config like sdcpp_args)
-        if (value.contains("recipe_options") && value["recipe_options"].is_object()) {
-            json_recipe_options[info.model_name] = value["recipe_options"];
-        }
-
         // Populate type and device fields (multi-model support)
         info.type = get_model_type_from_labels(info.labels);
         info.device = get_device_type_from_recipe(info.recipe);
@@ -2180,9 +2236,14 @@ void ModelManager::build_cache() {
 
     // Populate recipe options. recipe_options.json is keyed by canonical ID
     // (user.*, extra.*, builtin.*) — built-ins are keyed bare in the cache, so
-    // we translate before lookup.
+    // we translate before lookup. The per-model JSON recipe_options are read
+    // inline from the model entry (mirrors add_model_to_cache) rather than via a
+    // prebuilt map.
     for (auto& [name, info] : all_models) {
-        json jro = json_recipe_options.count(name) ? json_recipe_options[name] : json(nullptr);
+        json jro = json(nullptr);
+        if (const auto* model_json = find_model_json_entry(server_models_, user_models_, name)) {
+            jro = extract_json_recipe_options(model_json);
+        }
         info.recipe_options = build_recipe_options(info, jro, cache_key_to_canonical_id(name), recipe_options_);
     }
 
@@ -2240,20 +2301,8 @@ void ModelManager::add_model_to_cache(const std::string& model_name) {
         return; // Will initialize on next access
     }
 
-    // Parse model name to get JSON key
-    std::string json_key = model_name;
     bool is_user_model = is_user_model_name(model_name);
-    if (is_user_model) {
-        json_key = strip_user_model_prefix(model_name);
-    }
-
-    // Find in JSON
-    json* model_json = nullptr;
-    if (is_user_model && user_models_.contains(json_key)) {
-        model_json = &user_models_[json_key];
-    } else if (!is_user_model && server_models_.contains(json_key)) {
-        model_json = &server_models_[json_key];
-    }
+    const json* model_json = find_model_json_entry(server_models_, user_models_, model_name);
 
     if (!model_json) {
         LOG(WARNING, "ModelManager") << "'" << model_name << "' not found in JSON" << std::endl;
@@ -2271,8 +2320,7 @@ void ModelManager::add_model_to_cache(const std::string& model_name) {
     info.cloud_provider = JsonUtils::get_or_default<std::string>(*model_json, "cloud_provider", "");
 
     parse_image_defaults(info, *model_json);
-    json jro = (model_json->contains("recipe_options") && (*model_json)["recipe_options"].is_object())
-        ? (*model_json)["recipe_options"] : json(nullptr);
+    json jro = extract_json_recipe_options(model_json);
     info.recipe_options = build_recipe_options(info, jro, cache_key_to_canonical_id(model_name), recipe_options_);
 
     info.suggested = JsonUtils::get_or_default<bool>(*model_json, "suggested", is_user_model);
@@ -4946,6 +4994,7 @@ void ModelManager::delete_model(const std::string& model_name) {
             save_user_models(updated_user_models);
             user_models_ = std::move(updated_user_models);
             cache_valid_ = false;
+            delete_saved_model_options(canonical_model_name);
             LOG(INFO, "ModelManager") << "✓ Removed from user_models.json" << std::endl;
         }
 
@@ -4980,6 +5029,7 @@ void ModelManager::delete_model(const std::string& model_name) {
             save_user_models(updated_user_models);
             user_models_ = std::move(updated_user_models);
             cache_valid_ = false;
+            delete_saved_model_options(canonical_model_name);
             LOG(INFO, "ModelManager") << "✓ Removed from user_models.json" << std::endl;
         }
 
@@ -5076,6 +5126,7 @@ void ModelManager::delete_model(const std::string& model_name) {
         save_user_models(updated_user_models);
         user_models_ = std::move(updated_user_models);
         cache_valid_ = false;
+        delete_saved_model_options(canonical_model_name);
         LOG(INFO, "ModelManager") << "✓ Removed from user_models.json" << std::endl;
     }
 

src/cpp/server/server.cpp

@@ -3520,9 +3520,14 @@ void Server::handle_load(const httplib::Request& req, httplib::Response& res) {
         LOG(INFO, "Server") << " " << options.to_log_string(false);
         LOG(INFO, "Server") << std::endl;
 
-        // Persist request options to model info if requested
+        // Persist request options to model info if requested. Merge the request over
+        // the model's PRIOR persisted options (Layer 3) only — this preserves earlier
+        // user saves without baking image_defaults / model-JSON defaults (Layers 1-2)
+        // into recipe_options.json, which would shadow future registry default changes.
         if (save_options) {
-            info.recipe_options = options;
+            RecipeOptions prior_saved = model_manager_->get_saved_model_options(
+                info, /*refresh_saved_from_disk=*/true);
+            info.recipe_options = options.inherit(prior_saved);
             model_manager_->save_model_options(info);
         }