patch: Patch/ygo 25th set 2 (#14)

core/include/ccm/infra/CprHttpClient.hpp

@@ -24,11 +24,21 @@ namespace ccm {
 // only fires one outbound request at a time anyway.
 class CprHttpClient final : public IHttpClient {
 public:
+    struct RawResponse {
+        bool        transportError{false};
+        std::string transportMessage;
+        int         statusCode{0};
+        std::string body;
+    };
+
     using GetExecutor = std::function<Result<std::string>(std::string_view)>;
+    using RawGetExecutor = std::function<RawResponse(std::string_view)>;
 
     explicit CprHttpClient(std::chrono::milliseconds timeout = std::chrono::milliseconds{30000});
     CprHttpClient(GetExecutor executor,
                   std::chrono::milliseconds timeout = std::chrono::milliseconds{30000});
+    CprHttpClient(RawGetExecutor rawExecutor,
+                  std::chrono::milliseconds timeout = std::chrono::milliseconds{30000});
     ~CprHttpClient() override;
 
     Result<std::string> get(std::string_view url) override;
@@ -37,6 +47,7 @@ private:
     std::chrono::milliseconds timeout_;
     std::unique_ptr<cpr::Session> session_;
     GetExecutor executor_;
+    RawGetExecutor rawExecutor_;
     std::mutex sessionMutex_;
 };
 

core/src/games/yugioh/YuGiOhCardPreviewSource.cpp

@@ -31,6 +31,17 @@ std::string toLower(std::string s) {
     return s;
 }
 
+std::string canonicalizeSetNameForAutoDetect(std::string_view setName) {
+    std::string canonical = trim(std::string(setName));
+    constexpr std::string_view k25thSuffix = " (25th Anniversary Edition)";
+    if (canonical.size() > k25thSuffix.size()
+        && canonical.ends_with(k25thSuffix)) {
+        canonical.erase(canonical.size() - k25thSuffix.size());
+        canonical = trim(std::move(canonical));
+    }
+    return canonical;
+}
+
 // Pull the standard art URL out of a YGOPRODeck card object. We deliberately
 // always return card_images[0]: when no `cardset=` filter is applied, that
 // slot is the original/standard artwork (alt-art passcodes follow), which is
@@ -366,7 +377,7 @@ Result<std::vector<AutoDetectedPrint>> YuGiOhCardPreviewSource::parsePrintVarian
         if (!j.contains("data") || !j.at("data").is_array() || j.at("data").empty()) {
             return R::err("YGOPRODeck returned no matching cards.");
         }
-        const std::string wantedSet = trim(std::string(preferredSetName));
+        const std::string wantedSet = canonicalizeSetNameForAutoDetect(preferredSetName);
         const std::string wantedNameLower = toLower(trim(std::string(wantedCardName)));
 
         std::vector<AutoDetectedPrint> collected;
@@ -532,15 +543,16 @@ Result<std::vector<AutoDetectedPrint>> YuGiOhCardPreviewSource::detectPrintVaria
     std::string_view name,
     std::string_view setId) {
     using R = Result<std::vector<AutoDetectedPrint>>;
-    const std::string url = buildSearchUrl(name, setId);
+    const std::string canonicalSetName = canonicalizeSetNameForAutoDetect(setId);
+    const std::string url = buildSearchUrl(name, canonicalSetName);
     auto resp = http_.get(url);
     if (resp) {
-        return parsePrintVariants(resp.value(), setId, name);
+        return parsePrintVariants(resp.value(), canonicalSetName, name);
     }
     const std::string fallbackUrl = buildSearchUrl(name, "");
     auto fallback = http_.get(fallbackUrl);
     if (!fallback) return R::err(fallback.error());
-    return parsePrintVariants(fallback.value(), setId, name);
+    return parsePrintVariants(fallback.value(), canonicalSetName, name);
 }
 
 }  // namespace ccm

core/src/games/yugioh/YuGiOhSetSource.cpp

@@ -3,9 +3,43 @@
 #include <nlohmann/json.hpp>
 
 #include <algorithm>
+#include <array>
 #include <string>
 
 namespace ccm {
+namespace {
+
+struct YuGiOhSetAlias {
+    const char* code;
+    const char* name;
+    const char* releaseDate;
+};
+
+constexpr std::array<YuGiOhSetAlias, 6> kMissing25thAnniversaryReprints{{
+    // Keep this list in sync with docs/assets-and-info-apis.md (Info API section).
+    {"LOB-25TH", "Legend of Blue Eyes White Dragon (25th Anniversary Edition)", "2023/04/20"},
+    {"MRD-25TH", "Metal Raiders (25th Anniversary Edition)", "2023/04/20"},
+    {"SRL-25TH", "Spell Ruler (25th Anniversary Edition)", "2023/04/20"},
+    {"PSV-25TH", "Pharaoh's Servant (25th Anniversary Edition)", "2023/04/20"},
+    {"DCR-25TH", "Dark Crisis (25th Anniversary Edition)", "2023/04/20"},
+    {"IOC-25TH", "Invasion of Chaos (25th Anniversary Edition)", "2023/06/08"},
+}};
+
+void appendMissingSetAliases(std::vector<Set>& sets) {
+    for (const auto& alias : kMissing25thAnniversaryReprints) {
+        const bool exists = std::any_of(
+            sets.begin(), sets.end(), [&](const Set& s) { return s.name == alias.name; });
+        if (exists) continue;
+
+        Set s;
+        s.id = alias.code;
+        s.name = alias.name;
+        s.releaseDate = alias.releaseDate;
+        sets.push_back(std::move(s));
+    }
+}
+
+}  // namespace
 
 YuGiOhSetSource::YuGiOhSetSource(IHttpClient& http) : http_(http) {}
 
@@ -29,6 +63,7 @@ Result<std::vector<Set>> YuGiOhSetSource::parseResponse(const std::string& body)
             s.releaseDate = std::move(release);
             out.push_back(std::move(s));
         }
+        appendMissingSetAliases(out);
         std::sort(out.begin(), out.end(),
                   [](const Set& a, const Set& b) { return a.releaseDate < b.releaseDate; });
         return Result<std::vector<Set>>::ok(std::move(out));

core/src/infra/CprHttpClient.cpp

@@ -26,16 +26,15 @@ CprHttpClient::CprHttpClient(std::chrono::milliseconds timeout)
                                         /*follow=*/true,
                                         /*cont_send_cred=*/false,
                                         cpr::PostRedirectFlags::POST_ALL});
-    executor_ = [this](std::string_view url) -> Result<std::string> {
+    rawExecutor_ = [this](std::string_view url) -> RawResponse {
         session_->SetUrl(cpr::Url{std::string(url)});
         cpr::Response r = session_->Get();
-
-        if (r.error) {
-            return mapHttpGetResponse(true, r.error.message, r.status_code, {},
-                                      url);
-        }
-        return mapHttpGetResponse(false, {}, r.status_code, std::move(r.text),
-                                  url);
+        return RawResponse{
+            .transportError = static_cast<bool>(r.error),
+            .transportMessage = r.error.message,
+            .statusCode = static_cast<int>(r.status_code),
+            .body = std::move(r.text),
+        };
     };
 }
 
@@ -45,6 +44,12 @@ CprHttpClient::CprHttpClient(GetExecutor executor,
       session_(nullptr),
       executor_(std::move(executor)) {}
 
+CprHttpClient::CprHttpClient(RawGetExecutor rawExecutor,
+                             std::chrono::milliseconds timeout)
+    : timeout_(timeout),
+      session_(nullptr),
+      rawExecutor_(std::move(rawExecutor)) {}
+
 CprHttpClient::~CprHttpClient() = default;
 
 Result<std::string> CprHttpClient::get(std::string_view url) {
@@ -53,10 +58,18 @@ Result<std::string> CprHttpClient::get(std::string_view url) {
     // (one fetch per BaseSelectedCardPanel selection change), so contention
     // is negligible.
     std::lock_guard<std::mutex> lock(sessionMutex_);
-    if (!executor_) {
-        return Result<std::string>::err("HTTP error: no executor configured");
+    if (executor_) {
+        return executor_(url);
     }
-    return executor_(url);
+    if (rawExecutor_) {
+        RawResponse raw = rawExecutor_(url);
+        return mapHttpGetResponse(raw.transportError,
+                                  raw.transportMessage,
+                                  raw.statusCode,
+                                  std::move(raw.body),
+                                  url);
+    }
+    return Result<std::string>::err("HTTP error: no executor configured");
 }
 
 }  // namespace ccm

docs/assets-and-info-apis.md

@@ -39,6 +39,8 @@ Upstream documentation:
 `https://db.ygoprodeck.com/api/v7/cardsets.php`  
 Used by `YuGiOhSetSource`. The response is a top-level JSON array. Each object maps `set_code` → internal `Set.id`, `set_name` → `Set.name`, and `tcg_date` → `Set.releaseDate` with `-` rewritten to `/` for consistency with other games’ date strings. Results are sorted ascending by `releaseDate`.
 
+CCM3 also applies a deterministic local patch step in `YuGiOhSetSource::appendMissingSetAliases(...)` after parsing: if upstream omits known 25th Anniversary TCG reprints, the app injects missing aliases for `LOB-25TH`, `MRD-25TH`, `SRL-25TH`, `PSV-25TH`, `DCR-25TH`, and `IOC-25TH` (with fixed release dates) so users can still select those products in the set picker.
+
 ### Asset API: Yugipedia `api.php` (primary)
 
 `https://yugipedia.com/api.php?action=query&prop=imageinfo&iiprop=url&titles=...`  

tests/CMakeLists.txt

@@ -21,6 +21,7 @@ add_executable(ccm_core_tests
     std_file_system_tests.cpp
     pokemon_set_source_tests.cpp
     pokemon_card_preview_source_tests.cpp
+    icard_preview_source_tests.cpp
     yugioh_set_source_tests.cpp
     yugioh_card_preview_source_tests.cpp
     game_module_tests.cpp

tests/card_preview_service_tests.cpp

@@ -703,6 +703,40 @@ TEST_SUITE("CardPreviewService caching") {
         CHECK(http.calls == 1);
     }
 
+    TEST_CASE("fetchImageBytesByUrl propagates HTTP errors when uncached") {
+        FixedHttpClient http;
+        http.ok = false;
+        http.err = "url fetch failed";
+        CardPreviewService svc{http};
+
+        const auto out = svc.fetchImageBytesByUrl("https://cdn.example/back.png");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "url fetch failed");
+    }
+
+    TEST_CASE("fetchImageBytesByUrl serves from persistent cache hit without HTTP") {
+        FixedHttpClient http;
+        http.body = "warm-card-back";
+        InMemoryByteCache disk;
+
+        // Seed persistent cache via first service instance.
+        {
+            CardPreviewService seed{http, &disk};
+            const auto seeded = seed.fetchImageBytesByUrl("https://cdn.example/back.png");
+            REQUIRE(seeded.isOk());
+            CHECK(seeded.value() == "warm-card-back");
+        }
+        REQUIRE(http.calls == 1);
+
+        // Fresh service instance: force HTTP failure and ensure disk hit is used.
+        CardPreviewService warm{http, &disk};
+        http.ok = false;
+        const auto warmHit = warm.fetchImageBytesByUrl("https://cdn.example/back.png");
+        REQUIRE(warmHit.isOk());
+        CHECK(warmHit.value() == "warm-card-back");
+        CHECK(http.calls == 1);
+    }
+
     TEST_CASE("in-memory LRU evicts oldest entry after exceeding capacity") {
         FakeSource source;
         FakeGameModule module;

tests/collection_service_tests.cpp

@@ -16,9 +16,15 @@ namespace {
 class InMemoryRepo final : public ICollectionRepository<MagicCard> {
 public:
     Map storage;
+    bool failLoad{false};
+    bool failSave{false};
 
-    Result<Map> load(Game) override { return Result<Map>::ok(storage); }
+    Result<Map> load(Game) override {
+        if (failLoad) return Result<Map>::err("load failed");
+        return Result<Map>::ok(storage);
+    }
     Result<void> save(Game, const Map& m) override {
+        if (failSave) return Result<void>::err("save failed");
         storage = m;
         return Result<void>::ok();
     }
@@ -27,12 +33,14 @@ public:
 class StubImageStore final : public IImageStore {
 public:
     std::vector<std::pair<Game, std::string>> removed;
+    bool failRemove{false};
 
     Result<std::string> copyIn(Game, const std::filesystem::path&, const std::string& n) override {
         return Result<std::string>::ok(n);
     }
     Result<void> remove(Game g, const std::string& n) override {
         removed.emplace_back(g, n);
+        if (failRemove) return Result<void>::err("remove failed for " + n);
         return Result<void>::ok();
     }
     std::filesystem::path resolvePath(Game, const std::string& n) const override {
@@ -51,6 +59,16 @@ MagicCard makeCard(const std::string& name, std::vector<std::string> imgs = {})
 }  // namespace
 
 TEST_SUITE("CollectionService<MagicCard>") {
+    TEST_CASE("nextId uses highest existing id plus one") {
+        InMemoryRepo repo;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        repo.storage.emplace(2, makeCard("A"));
+        repo.storage.emplace(9, makeCard("B"));
+        CHECK(CollectionService<MagicCard>::nextId(repo.storage) == 10);
+    }
+
     TEST_CASE("nextId on empty map is 0, then strictly increments") {
         InMemoryRepo repo;
         StubImageStore store;
@@ -117,4 +135,105 @@ TEST_SUITE("CollectionService<MagicCard>") {
 
         CHECK(svc.remove(Game::Magic, 12345).isErr());
     }
+
+    TEST_CASE("load errors are propagated by list and findById") {
+        InMemoryRepo repo;
+        repo.failLoad = true;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto listed = svc.list(Game::Magic);
+        REQUIRE(listed.isErr());
+        CHECK(listed.error() == "load failed");
+
+        const auto found = svc.findById(Game::Magic, 1);
+        REQUIRE(found.isErr());
+        CHECK(found.error() == "load failed");
+    }
+
+    TEST_CASE("save errors are propagated by add and update") {
+        InMemoryRepo repo;
+        repo.failSave = true;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto addRes = svc.add(Game::Magic, makeCard("A"));
+        REQUIRE(addRes.isErr());
+        CHECK(addRes.error() == "save failed");
+
+        repo.failSave = false;
+        const auto id = svc.add(Game::Magic, makeCard("B"));
+        REQUIRE(id.isOk());
+
+        repo.failSave = true;
+        MagicCard updated = makeCard("Renamed");
+        updated.id = id.value();
+        const auto updateRes = svc.update(Game::Magic, updated);
+        REQUIRE(updateRes.isErr());
+        CHECK(updateRes.error() == "save failed");
+    }
+
+    TEST_CASE("add overwrites input card id with generated id") {
+        InMemoryRepo repo;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        MagicCard card = makeCard("Has User Id");
+        card.id = 777;
+        const auto out = svc.add(Game::Magic, card);
+        REQUIRE(out.isOk());
+        CHECK(out.value() == 0);
+        REQUIRE(repo.storage.count(0) == 1);
+        CHECK(repo.storage.at(0).name == "Has User Id");
+        CHECK(repo.storage.count(777) == 0);
+    }
+
+    TEST_CASE("findById returns nullopt for missing id") {
+        InMemoryRepo repo;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto out = svc.findById(Game::Magic, 99);
+        REQUIRE(out.isOk());
+        CHECK_FALSE(out.value().has_value());
+    }
+
+    TEST_CASE("remove reports image cleanup issues but still removes card") {
+        InMemoryRepo repo;
+        StubImageStore store;
+        store.failRemove = true;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto id = svc.add(
+            Game::Magic, makeCard("With Images", {"a.png", "b.png"}));
+        REQUIRE(id.isOk());
+
+        const auto removed = svc.remove(Game::Magic, id.value());
+        REQUIRE(removed.isErr());
+        CHECK(removed.error().find("Card removed but image cleanup had issues:") != std::string::npos);
+        CHECK(removed.error().find("remove failed for a.png") != std::string::npos);
+        CHECK(removed.error().find("remove failed for b.png") != std::string::npos);
+
+        const auto listed = svc.list(Game::Magic);
+        REQUIRE(listed.isOk());
+        CHECK(listed.value().empty());
+    }
+
+    TEST_CASE("remove propagates save failure after image cleanup") {
+        InMemoryRepo repo;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto id = svc.add(
+            Game::Magic, makeCard("With Images", {"a.png", "b.png"}));
+        REQUIRE(id.isOk());
+
+        repo.failSave = true;
+        const auto removed = svc.remove(Game::Magic, id.value());
+        REQUIRE(removed.isErr());
+        CHECK(removed.error() == "save failed");
+        REQUIRE(store.removed.size() == 2);
+        CHECK(store.removed[0].second == "a.png");
+        CHECK(store.removed[1].second == "b.png");
+    }
 }

tests/cpr_http_client_tests.cpp

@@ -34,4 +34,113 @@ TEST_SUITE("CprHttpClient injected executor") {
         REQUIRE(out.isErr());
         CHECK(out.error() == "HTTP 503 from https://example.com");
     }
+
+    TEST_CASE("returns an explicit error when executor is empty") {
+        CprHttpClient::GetExecutor empty;
+        CprHttpClient client{empty};
+
+        const auto out = client.get("https://example.com");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "HTTP error: no executor configured");
+    }
+}
+
+TEST_SUITE("CprHttpClient injected raw executor") {
+    TEST_CASE("maps 2xx raw response to success body") {
+        CprHttpClient client{
+            [](std::string_view) -> CprHttpClient::RawResponse {
+                return CprHttpClient::RawResponse{
+                    .transportError = false,
+                    .transportMessage = "",
+                    .statusCode = 200,
+                    .body = "ok-body",
+                };
+            }
+        };
+
+        const auto out = client.get("https://example.com/success");
+        REQUIRE(out.isOk());
+        CHECK(out.value() == "ok-body");
+    }
+
+    TEST_CASE("maps transport error via shared http mapping") {
+        CprHttpClient client{
+            [](std::string_view) -> CprHttpClient::RawResponse {
+                return CprHttpClient::RawResponse{
+                    .transportError = true,
+                    .transportMessage = "timeout",
+                    .statusCode = 0,
+                    .body = "",
+                };
+            }
+        };
+
+        const auto out = client.get("https://example.com/timeout");
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("timeout") != std::string::npos);
+    }
+
+    TEST_CASE("passes URL through raw executor unchanged") {
+        std::string seenUrl;
+        CprHttpClient client{
+            [&seenUrl](std::string_view url) -> CprHttpClient::RawResponse {
+                seenUrl = std::string(url);
+                return CprHttpClient::RawResponse{
+                    .transportError = false,
+                    .transportMessage = "",
+                    .statusCode = 200,
+                    .body = "ok",
+                };
+            }
+        };
+
+        const auto out = client.get("https://example.com/raw?q=a%20b");
+        REQUIRE(out.isOk());
+        CHECK(seenUrl == "https://example.com/raw?q=a%20b");
+    }
+
+    TEST_CASE("maps non-2xx status to error") {
+        CprHttpClient client{
+            [](std::string_view) -> CprHttpClient::RawResponse {
+                return CprHttpClient::RawResponse{
+                    .transportError = false,
+                    .transportMessage = "",
+                    .statusCode = 503,
+                    .body = "service unavailable",
+                };
+            }
+        };
+
+        const auto out = client.get("https://example.com/fail");
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("HTTP 503") != std::string::npos);
+    }
+
+    TEST_CASE("transport error takes precedence over status code") {
+        CprHttpClient client{
+            [](std::string_view) -> CprHttpClient::RawResponse {
+                return CprHttpClient::RawResponse{
+                    .transportError = true,
+                    .transportMessage = "socket closed",
+                    .statusCode = 200,
+                    .body = "ignored",
+                };
+            }
+        };
+
+        const auto out = client.get("https://example.com/transport");
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("socket closed") != std::string::npos);
+    }
+}
+
+TEST_SUITE("CprHttpClient real session") {
+    TEST_CASE("default constructor handles malformed URL without crashing") {
+        // Exercise the real cpr::Session-backed constructor/lambda path
+        // without depending on external network availability.
+        CprHttpClient client{};
+        const auto out = client.get("://not-a-valid-url");
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("HTTP") != std::string::npos);
+    }
 }

tests/domain_json_tests.cpp

@@ -290,6 +290,29 @@ TEST_SUITE("Domain JSON required fields") {
         CHECK_THROWS(j.get<PokemonCard>());
     }
 
+    TEST_CASE("YuGiOhCard missing required key throws") {
+        const nlohmann::json j = {
+            {"id", 7},
+            {"amount", 1},
+            {"name", "Blue-Eyes White Dragon"},
+            {"set", nlohmann::json{
+                {"id", "sdk"},
+                {"name", "Starter Deck Kaiba"},
+                {"releaseDate", "2002/03/29"},
+            }},
+            {"setNo", "SDK-001"},
+            {"note", ""},
+            {"images", nlohmann::json::array()},
+            {"language", "English"},
+            {"condition", "NearMint"},
+            {"firstEdition", true},
+            // rarity missing on purpose
+            {"signed", false},
+            {"altered", false},
+        };
+        CHECK_THROWS(j.get<YuGiOhCard>());
+    }
+
     TEST_CASE("Configuration missing required key throws") {
         const nlohmann::json j = {
             {"defaultGame", "Magic"},
@@ -297,4 +320,13 @@ TEST_SUITE("Domain JSON required fields") {
         };
         CHECK_THROWS(j.get<Configuration>());
     }
+
+    TEST_CASE("Configuration invalid theme value throws when present") {
+        const nlohmann::json j = {
+            {"dataStorage", "/portable/data"},
+            {"defaultGame", "Magic"},
+            {"theme", "Neon"},
+        };
+        CHECK_THROWS(j.get<Configuration>());
+    }
 }

tests/icard_preview_source_tests.cpp

@@ -0,0 +1,41 @@
+#include <doctest/doctest.h>
+
+#include "ccm/ports/ICardPreviewSource.hpp"
+
+using namespace ccm;
+
+namespace {
+
+class MinimalPreviewSource final : public ICardPreviewSource {
+public:
+    Result<std::string, PreviewLookupError>
+        fetchImageUrl(std::string_view,
+                      std::string_view,
+                      std::string_view) override {
+        return Result<std::string, PreviewLookupError>::err(
+            PreviewLookupError{PreviewLookupError::Kind::NotFound, "not found"});
+    }
+};
+
+}  // namespace
+
+TEST_SUITE("ICardPreviewSource defaults") {
+    TEST_CASE("auto-detect is disabled by default") {
+        MinimalPreviewSource src;
+        CHECK_FALSE(src.supportsAutoDetectPrint());
+    }
+
+    TEST_CASE("default detectFirstPrint returns explicit unsupported error") {
+        MinimalPreviewSource src;
+        const auto out = src.detectFirstPrint("Card", "Set");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "Auto-detect not supported by this game.");
+    }
+
+    TEST_CASE("default detectPrintVariants returns explicit unsupported error") {
+        MinimalPreviewSource src;
+        const auto out = src.detectPrintVariants("Card", "Set");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "Print variant listing not supported by this game.");
+    }
+}

tests/image_service_tests.cpp

@@ -19,16 +19,24 @@ public:
     std::vector<Call> copies;
     std::vector<std::pair<Game, std::string>> removes;
     std::string returnedExt = ".png";
+    int failCopyAt = -1;
+    int failRemoveAt = -1;
 
     Result<std::string> copyIn(Game game,
                                const std::filesystem::path& srcPath,
                                const std::string& targetName) override {
         copies.push_back({game, srcPath, targetName});
+        if (failCopyAt >= 0 && static_cast<int>(copies.size()) == failCopyAt) {
+            return Result<std::string>::err("copy failed at " + std::to_string(failCopyAt));
+        }
         return Result<std::string>::ok(targetName + returnedExt);
     }
 
     Result<void> remove(Game game, const std::string& imageName) override {
         removes.emplace_back(game, imageName);
+        if (failRemoveAt >= 0 && static_cast<int>(removes.size()) == failRemoveAt) {
+            return Result<void>::err("remove failed at " + std::to_string(failRemoveAt));
+        }
         return Result<void>::ok();
     }
 
@@ -55,6 +63,16 @@ TEST_SUITE("ImageService::nextImageIndex") {
         std::vector<std::string> imgs2 = {"otherIMG_BACK.png"};
         CHECK(ImageService::nextImageIndex(imgs2) == 0);
     }
+
+    TEST_CASE("index increments from two-digit legacy cap") {
+        std::vector<std::string> imgs = {"set+name+99.png"};
+        CHECK(ImageService::nextImageIndex(imgs) == 100);
+    }
+
+    TEST_CASE("three-digit filename index follows two-digit compatibility parser") {
+        std::vector<std::string> imgs = {"set+name+255.png"};
+        CHECK(ImageService::nextImageIndex(imgs) == 56);
+    }
 }
 
 TEST_SUITE("ImageService::buildTargetName") {
@@ -85,6 +103,40 @@ TEST_SUITE("ImageService::addImage") {
         CHECK(store.copies[0].game == Game::Magic);
         CHECK(store.copies[0].target == "Beta+BlackLotus+0");
     }
+
+    TEST_CASE("propagates copyIn failures") {
+        RecordingImageStore store;
+        store.failCopyAt = 1;
+        ImageService svc{store};
+
+        std::vector<std::string> existing;
+        const auto out = svc.addImage(Game::Magic, "/tmp/source.png",
+                                      /*newEntry=*/true, /*cardId=*/0,
+                                      "Beta", "Black Lotus", existing);
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("copy failed at 1") != std::string::npos);
+    }
+}
+
+TEST_SUITE("ImageService::removeImage and resolveImagePath") {
+    TEST_CASE("removeImage delegates to store remove") {
+        RecordingImageStore store;
+        ImageService svc{store};
+
+        const auto out = svc.removeImage(Game::Magic, "x.png");
+        REQUIRE(out.isOk());
+        REQUIRE(store.removes.size() == 1);
+        CHECK(store.removes[0].first == Game::Magic);
+        CHECK(store.removes[0].second == "x.png");
+    }
+
+    TEST_CASE("resolveImagePath delegates to store resolvePath") {
+        RecordingImageStore store;
+        ImageService svc{store};
+
+        const auto p = svc.resolveImagePath(Game::Pokemon, "pikachu.jpg");
+        CHECK(p == std::filesystem::path("/fake/pikachu.jpg"));
+    }
 }
 
 TEST_SUITE("ImageService::normalizeNamesForPersistedCard") {
@@ -128,4 +180,52 @@ TEST_SUITE("ImageService::normalizeNamesForPersistedCard") {
         CHECK(store.copies.empty());
         CHECK(store.removes.empty());
     }
+
+    TEST_CASE("skips rename when computed output name equals input") {
+        RecordingImageStore store;
+        ImageService svc{store};
+
+        const std::vector<std::string> images{"42+Beta+BlackLotus+0.png"};
+        auto normalized = svc.normalizeNamesForPersistedCard(
+            Game::Magic, 42, "Beta", "Black Lotus", images);
+
+        REQUIRE(normalized.isOk());
+        CHECK(normalized.value() == images);
+        CHECK(store.copies.empty());
+        CHECK(store.removes.empty());
+    }
+
+    TEST_CASE("copy failure rolls back already-created names and returns error") {
+        RecordingImageStore store;
+        store.failCopyAt = 2;
+        ImageService svc{store};
+
+        const std::vector<std::string> images{
+            "Beta+BlackLotus+0.png",
+            "Beta+BlackLotus+1.jpg"
+        };
+        const auto out = svc.normalizeNamesForPersistedCard(
+            Game::Magic, 42, "Beta", "Black Lotus", images);
+
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("copy failed at 2") != std::string::npos);
+        // Second copy failed, so first created file should be rolled back.
+        REQUIRE(store.removes.size() == 1);
+        CHECK(store.removes[0].second == "42+Beta+BlackLotus+0.png");
+    }
+
+    TEST_CASE("remove failure after rename returns error") {
+        RecordingImageStore store;
+        store.failRemoveAt = 1;
+        ImageService svc{store};
+
+        const std::vector<std::string> images{
+            "Beta+BlackLotus+0.png"
+        };
+        const auto out = svc.normalizeNamesForPersistedCard(
+            Game::Magic, 42, "Beta", "Black Lotus", images);
+
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("remove failed at 1") != std::string::npos);
+    }
 }

tests/json_collection_repository_tests.cpp

@@ -8,6 +8,8 @@
 
 #include <nlohmann/json.hpp>
 
+#include <filesystem>
+
 using namespace ccm;
 using ccm::testing::InMemoryFileSystem;
 
@@ -27,6 +29,37 @@ ConfigService makeConfig(InMemoryFileSystem& fs, const std::string& dataDir) {
 
 std::string magicDir(Game g) { return g == Game::Magic ? "magic" : "pokemon"; }
 
+class FailingCollectionFs final : public IFileSystem {
+public:
+    bool existsValue{true};
+    bool ensureOk{true};
+    bool writeOk{true};
+    bool readOk{true};
+    std::string readPayload{"{}"};
+
+    [[nodiscard]] bool exists(const std::filesystem::path&) const override { return existsValue; }
+    [[nodiscard]] bool isDirectory(const std::filesystem::path&) const override { return true; }
+    Result<void> ensureDirectory(const std::filesystem::path&) override {
+        if (!ensureOk) return Result<void>::err("ensure failed");
+        return Result<void>::ok();
+    }
+    Result<std::string> readText(const std::filesystem::path&) override {
+        if (!readOk) return Result<std::string>::err("read failed");
+        return Result<std::string>::ok(readPayload);
+    }
+    Result<void> writeText(const std::filesystem::path&, std::string_view) override {
+        if (!writeOk) return Result<void>::err("write failed");
+        return Result<void>::ok();
+    }
+    Result<void> copyFile(const std::filesystem::path&, const std::filesystem::path&, bool) override {
+        return Result<void>::ok();
+    }
+    Result<void> remove(const std::filesystem::path&) override { return Result<void>::ok(); }
+    Result<std::vector<std::filesystem::path>> listDirectory(const std::filesystem::path&) override {
+        return Result<std::vector<std::filesystem::path>>::ok({});
+    }
+};
+
 }  // namespace
 
 TEST_SUITE("JsonCollectionRepository<MagicCard>") {
@@ -85,4 +118,49 @@ TEST_SUITE("JsonCollectionRepository<MagicCard>") {
         REQUIRE(j.contains("17"));
         CHECK(j.at("17").at("id") == 17);
     }
+
+    TEST_CASE("load returns parse error for non-object root") {
+        InMemoryFileSystem fs;
+        auto cfg = makeConfig(fs, "/data");
+        JsonCollectionRepository<MagicCard> repo{fs, cfg, magicDir};
+        fs.writeText("/data/magic/collection.json", R"(["not","an","object"])");
+
+        const auto loaded = repo.load(Game::Magic);
+        REQUIRE(loaded.isErr());
+        CHECK(loaded.error().find("JSON parse error:") != std::string::npos);
+    }
+
+    TEST_CASE("load returns parse error for non-numeric object keys") {
+        InMemoryFileSystem fs;
+        auto cfg = makeConfig(fs, "/data");
+        JsonCollectionRepository<MagicCard> repo{fs, cfg, magicDir};
+        fs.writeText("/data/magic/collection.json", R"({"abc":{"id":1}})");
+
+        const auto loaded = repo.load(Game::Magic);
+        REQUIRE(loaded.isErr());
+        CHECK(loaded.error().find("JSON parse error:") != std::string::npos);
+    }
+
+    TEST_CASE("save and initialize-on-load propagate ensureDirectory/write errors") {
+        InMemoryFileSystem configFs;
+        auto cfg = makeConfig(configFs, "/data");
+        FailingCollectionFs fs;
+        JsonCollectionRepository<MagicCard> repo{fs, cfg, magicDir};
+
+        fs.ensureOk = false;
+        const auto saveEnsureFail = repo.save(Game::Magic, {});
+        REQUIRE(saveEnsureFail.isErr());
+        CHECK(saveEnsureFail.error() == "ensure failed");
+
+        fs.ensureOk = true;
+        fs.writeOk = false;
+        const auto saveWriteFail = repo.save(Game::Magic, {});
+        REQUIRE(saveWriteFail.isErr());
+        CHECK(saveWriteFail.error() == "write failed");
+
+        fs.existsValue = false;
+        const auto loadCreateFail = repo.load(Game::Magic);
+        REQUIRE(loadCreateFail.isErr());
+        CHECK(loadCreateFail.error() == "write failed");
+    }
 }

tests/json_set_repository_tests.cpp

@@ -22,6 +22,43 @@ ConfigService makeConfig(InMemoryFileSystem& fs, const std::string& dataDir) {
     cfg.initialize();
     return cfg;
 }
+
+class FailingSetFs final : public IFileSystem {
+public:
+    bool ensureOk{true};
+    bool writeOk{true};
+    bool readOk{true};
+    std::string readPayload{"[]"};
+    std::filesystem::path lastWritePath;
+    std::string lastWriteBody;
+    std::filesystem::path lastReadPath;
+
+    [[nodiscard]] bool exists(const std::filesystem::path&) const override { return true; }
+    [[nodiscard]] bool isDirectory(const std::filesystem::path&) const override { return true; }
+
+    Result<void> ensureDirectory(const std::filesystem::path&) override {
+        if (!ensureOk) return Result<void>::err("ensure failed");
+        return Result<void>::ok();
+    }
+    Result<std::string> readText(const std::filesystem::path&) override {
+        lastReadPath = std::filesystem::path("/tracked/read/path");
+        if (!readOk) return Result<std::string>::err("read failed");
+        return Result<std::string>::ok(readPayload);
+    }
+    Result<void> writeText(const std::filesystem::path& p, std::string_view contents) override {
+        if (!writeOk) return Result<void>::err("write failed");
+        lastWritePath = p;
+        lastWriteBody = std::string(contents);
+        return Result<void>::ok();
+    }
+    Result<void> copyFile(const std::filesystem::path&, const std::filesystem::path&, bool) override {
+        return Result<void>::ok();
+    }
+    Result<void> remove(const std::filesystem::path&) override { return Result<void>::ok(); }
+    Result<std::vector<std::filesystem::path>> listDirectory(const std::filesystem::path&) override {
+        return Result<std::vector<std::filesystem::path>>::ok({});
+    }
+};
 }  // namespace
 
 TEST_SUITE("JsonSetRepository") {
@@ -49,4 +86,70 @@ TEST_SUITE("JsonSetRepository") {
         const auto loaded = repo.load(Game::Pokemon);
         CHECK(loaded.isErr());
     }
+
+    TEST_CASE("load propagates read errors from filesystem") {
+        InMemoryFileSystem configFs;
+        auto cfg = makeConfig(configFs, "/data");
+        FailingSetFs fs;
+        fs.readOk = false;
+        JsonSetRepository repo{fs, cfg, dirNameFn};
+
+        const auto loaded = repo.load(Game::Magic);
+        REQUIRE(loaded.isErr());
+        CHECK(loaded.error() == "read failed");
+    }
+
+    TEST_CASE("load reports parse error for malformed sets.json") {
+        InMemoryFileSystem configFs;
+        auto cfg = makeConfig(configFs, "/data");
+        FailingSetFs fs;
+        fs.readPayload = "{bad json";
+        JsonSetRepository repo{fs, cfg, dirNameFn};
+
+        const auto loaded = repo.load(Game::Magic);
+        REQUIRE(loaded.isErr());
+        CHECK(loaded.error().find("sets.json parse error:") != std::string::npos);
+    }
+
+    TEST_CASE("load reports parse error for wrong JSON shape") {
+        InMemoryFileSystem configFs;
+        auto cfg = makeConfig(configFs, "/data");
+        FailingSetFs fs;
+        fs.readPayload = R"({"not":"an array"})";
+        JsonSetRepository repo{fs, cfg, dirNameFn};
+
+        const auto loaded = repo.load(Game::Magic);
+        REQUIRE(loaded.isErr());
+        CHECK(loaded.error().find("sets.json parse error:") != std::string::npos);
+    }
+
+    TEST_CASE("save propagates ensureDirectory and writeText failures") {
+        InMemoryFileSystem configFs;
+        auto cfg = makeConfig(configFs, "/data");
+        FailingSetFs fs;
+        JsonSetRepository repo{fs, cfg, dirNameFn};
+
+        const std::vector<Set> sets = {{"lea", "Limited Edition Alpha", "1993/08/05"}};
+
+        fs.ensureOk = false;
+        const auto ensureFail = repo.save(Game::Magic, sets);
+        REQUIRE(ensureFail.isErr());
+        CHECK(ensureFail.error() == "ensure failed");
+
+        fs.ensureOk = true;
+        fs.writeOk = false;
+        const auto writeFail = repo.save(Game::Magic, sets);
+        REQUIRE(writeFail.isErr());
+        CHECK(writeFail.error() == "write failed");
+    }
+
+    TEST_CASE("paths are composed from dataStorage and game dir") {
+        InMemoryFileSystem fs;
+        auto cfg = makeConfig(fs, "/data");
+        JsonSetRepository repo{fs, cfg, dirNameFn};
+        const std::vector<Set> sets = {{"base1", "Base Set", "1999/01/09"}};
+
+        REQUIRE(repo.save(Game::Pokemon, sets).isOk());
+        CHECK(fs.files().count("/data/pokemon/sets.json") == 1);
+    }
 }

tests/local_image_store_tests.cpp

@@ -21,6 +21,37 @@ std::string dirNameForGame(Game g) {
     return "magic";
 }
 
+class FailingImageFs final : public IFileSystem {
+public:
+    bool ensureOk{true};
+    bool copyOk{true};
+    bool removeOk{true};
+
+    [[nodiscard]] bool exists(const std::filesystem::path&) const override { return true; }
+    [[nodiscard]] bool isDirectory(const std::filesystem::path&) const override { return true; }
+    Result<void> ensureDirectory(const std::filesystem::path&) override {
+        if (!ensureOk) return Result<void>::err("ensure failed");
+        return Result<void>::ok();
+    }
+    Result<std::string> readText(const std::filesystem::path&) override {
+        return Result<std::string>::ok({});
+    }
+    Result<void> writeText(const std::filesystem::path&, std::string_view) override {
+        return Result<void>::ok();
+    }
+    Result<void> copyFile(const std::filesystem::path&, const std::filesystem::path&, bool) override {
+        if (!copyOk) return Result<void>::err("copy failed");
+        return Result<void>::ok();
+    }
+    Result<void> remove(const std::filesystem::path&) override {
+        if (!removeOk) return Result<void>::err("remove failed");
+        return Result<void>::ok();
+    }
+    Result<std::vector<std::filesystem::path>> listDirectory(const std::filesystem::path&) override {
+        return Result<std::vector<std::filesystem::path>>::ok({});
+    }
+};
+
 }  // namespace
 
 TEST_SUITE("LocalImageStore") {
@@ -87,4 +118,30 @@ TEST_SUITE("LocalImageStore") {
         const std::filesystem::path got = store.resolvePath(Game::Pokemon, "pic.jpg");
         CHECK(got.generic_string() == "/coll/pokemon/images/pic.jpg");
     }
+
+    TEST_CASE("copyIn propagates ensureDirectory failure") {
+        InMemoryFileSystem configFs;
+        ConfigService cfg{configFs, "/app/config.json", "/coll"};
+        REQUIRE(cfg.initialize().isOk());
+        FailingImageFs fs;
+        fs.ensureOk = false;
+        LocalImageStore store(fs, cfg, dirNameForGame);
+
+        const auto out = store.copyIn(Game::Magic, "/incoming/a.png", "id001");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "ensure failed");
+    }
+
+    TEST_CASE("remove propagates filesystem remove failure when file exists") {
+        InMemoryFileSystem configFs;
+        ConfigService cfg{configFs, "/app/config.json", "/coll"};
+        REQUIRE(cfg.initialize().isOk());
+        FailingImageFs fs;
+        fs.removeOk = false;
+        LocalImageStore store(fs, cfg, dirNameForGame);
+
+        const auto out = store.remove(Game::Magic, "a.png");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "remove failed");
+    }
 }

tests/local_preview_byte_cache_tests.cpp

@@ -234,6 +234,55 @@ TEST_SUITE("LocalPreviewByteCache") {
         CHECK(cache.load("real-key").kind == IPreviewByteCache::HitKind::Miss);
     }
 
+    TEST_CASE("entry with payload but missing sidecar is treated as miss") {
+        TempDir td;
+        StdFileSystem fs;
+        LocalPreviewByteCache cache(fs, td.path);
+        cache.store("real-key", "REAL");
+
+        for (const auto& entry : fs::directory_iterator(td.path)) {
+            if (entry.path().extension() == ".idx") {
+                std::error_code ec;
+                fs::remove(entry.path(), ec);
+            }
+        }
+
+        CHECK(cache.load("real-key").kind == IPreviewByteCache::HitKind::Miss);
+    }
+
+    TEST_CASE("entry with negative marker but missing sidecar is treated as miss") {
+        TempDir td;
+        StdFileSystem fs;
+        LocalPreviewByteCache cache(fs, td.path);
+        cache.storeNegative("real-key");
+
+        for (const auto& entry : fs::directory_iterator(td.path)) {
+            if (entry.path().extension() == ".idx") {
+                std::error_code ec;
+                fs::remove(entry.path(), ec);
+            }
+        }
+
+        CHECK(cache.load("real-key").kind == IPreviewByteCache::HitKind::Miss);
+    }
+
+    TEST_CASE("entry with unreadable payload file is treated as miss") {
+        TempDir td;
+        StdFileSystem fs;
+        LocalPreviewByteCache cache(fs, td.path);
+        cache.store("real-key", "REAL");
+
+        for (const auto& entry : fs::directory_iterator(td.path)) {
+            if (entry.path().extension() == ".bin") {
+                std::error_code ec;
+                fs::remove(entry.path(), ec);
+                break;
+            }
+        }
+
+        CHECK(cache.load("real-key").kind == IPreviewByteCache::HitKind::Miss);
+    }
+
     TEST_CASE("evicts oldest entry when the size cap would be exceeded") {
         TempDir td;
         StdFileSystem fs;

tests/set_service_tests.cpp

@@ -39,11 +39,13 @@ class InMemSetRepo final : public ISetRepository {
 public:
     std::vector<Set> stored;
     bool hasStored = false;
+    bool failSave = false;
     Result<std::vector<Set>> load(Game) override {
         if (!hasStored) return Result<std::vector<Set>>::err("no cache");
         return Result<std::vector<Set>>::ok(stored);
     }
     Result<void> save(Game, const std::vector<Set>& s) override {
+        if (failSave) return Result<void>::err("save failed");
         stored = s;
         hasStored = true;
         return Result<void>::ok();
@@ -145,4 +147,36 @@ TEST_SUITE("SetService") {
         CHECK(pokemon.source.calls == 1);
         CHECK(yugioh.source.calls == 1);
     }
+
+    TEST_CASE("updateSets propagates repository save failures") {
+        InMemSetRepo repo;
+        repo.failSave = true;
+        SetService svc{repo};
+        FakeGameModule magic{Game::Magic};
+        magic.source.result = Result<std::vector<Set>>::ok({{"lea", "Alpha", "1993/08/05"}});
+        svc.registerModule(&magic);
+
+        const auto out = svc.updateSets(Game::Magic);
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "save failed");
+    }
+
+    TEST_CASE("registering a second module for same game id overwrites previous one") {
+        InMemSetRepo repo;
+        SetService svc{repo};
+        FakeGameModule firstMagic{Game::Magic};
+        firstMagic.source.result = Result<std::vector<Set>>::ok({{"a", "First", "2000/01/01"}});
+        FakeGameModule secondMagic{Game::Magic};
+        secondMagic.source.result = Result<std::vector<Set>>::ok({{"b", "Second", "2001/01/01"}});
+
+        svc.registerModule(&firstMagic);
+        svc.registerModule(&secondMagic);
+
+        const auto out = svc.updateSets(Game::Magic);
+        REQUIRE(out.isOk());
+        REQUIRE(out.value().size() == 1);
+        CHECK(out.value().front().id == "b");
+        CHECK(firstMagic.source.calls == 0);
+        CHECK(secondMagic.source.calls == 1);
+    }
 }

tests/std_file_system_tests.cpp

@@ -112,6 +112,22 @@ TEST_SUITE("StdFileSystem") {
         CHECK(r.value() == "hi");
     }
 
+    TEST_CASE("writeText/readText work for top-level relative files") {
+        TempDir td;
+        StdFileSystem fs;
+        const auto oldCwd = fs::current_path();
+        fs::current_path(td.path);
+
+        const fs::path topLevel = "top-level.txt";
+        REQUIRE(fs.writeText(topLevel, "hello").isOk());
+        const auto r = fs.readText(topLevel);
+        REQUIRE(r.isOk());
+        CHECK(r.value() == "hello");
+
+        std::error_code ec;
+        fs::current_path(oldCwd, ec);
+    }
+
     TEST_CASE("copyFile copies bytes and respects overwrite flag") {
         TempDir td;
         StdFileSystem fs;

tests/yugioh_card_preview_source_tests.cpp

@@ -446,6 +446,22 @@ TEST_SUITE("YuGiOhCardPreviewSource::parsePrintVariants") {
         REQUIRE(out.isErr());
         CHECK(out.error().find("YGOPRODeck JSON parse error") != std::string::npos);
     }
+
+    TEST_CASE("maps 25th Anniversary display-set alias to original set name") {
+        const std::string json = R"({
+          "data":[
+            {"name":"Dark Magician",
+             "card_sets":[
+               {"set_name":"Legend of Blue Eyes White Dragon","set_code":"LOB-005","set_rarity":"Ultra Rare"}
+             ]}
+          ]
+        })";
+        const auto out = YuGiOhCardPreviewSource::parsePrintVariants(
+            json, "Legend of Blue Eyes White Dragon (25th Anniversary Edition)", "Dark Magician");
+        REQUIRE(out.isOk());
+        REQUIRE(out.value().size() == 1);
+        CHECK(out.value()[0].setNo == "LOB-005");
+    }
 }
 
 TEST_SUITE("YuGiOhCardPreviewSource::detectPrintVariants HTTP fallback") {
@@ -467,6 +483,26 @@ TEST_SUITE("YuGiOhCardPreviewSource::detectPrintVariants HTTP fallback") {
         CHECK(out.value()[0].setNo == "MP21-EN001");
         REQUIRE(http.calls == 2);
     }
+
+    TEST_CASE("uses original set name in cardset query for 25th alias") {
+        FixedHttpClient http;
+        http.body = R"({
+          "data":[{
+            "name":"Dark Magician",
+            "card_sets":[
+              {"set_name":"Legend of Blue Eyes White Dragon","set_code":"LOB-005","set_rarity":"Ultra Rare"}
+            ]
+          }]
+        })";
+
+        YuGiOhCardPreviewSource src{http};
+        const auto out = src.detectPrintVariants(
+            "Dark Magician", "Legend of Blue Eyes White Dragon (25th Anniversary Edition)");
+        REQUIRE(out.isOk());
+        CHECK(http.lastUrl.find("cardset=Legend%20of%20Blue%20Eyes%20White%20Dragon")
+              != std::string::npos);
+        CHECK(http.lastUrl.find("25th") == std::string::npos);
+    }
 }
 
 // Helpers aligned with external fixture `yugioh_same_card_set_variant_tests`
@@ -642,6 +678,12 @@ TEST_SUITE("YuGiOhCardPreviewSource::parsePrintVariants yugioh_same_card_set_var
 }
 
 TEST_SUITE("YuGiOhCardPreviewSource::fetchImageUrl") {
+    TEST_CASE("supports auto-detect print metadata") {
+        FixedHttpClient http;
+        YuGiOhCardPreviewSource src{http};
+        CHECK(src.supportsAutoDetectPrint());
+    }
+
     TEST_CASE("queries Yugipedia first and uses the per-printing scan when found") {
         // Two same-passcode reprints with genuinely different art (LOB vs
         // SDK Blue-Eyes). Yugipedia hosts both, so we should always pick the
@@ -754,6 +796,34 @@ TEST_SUITE("YuGiOhCardPreviewSource::fetchImageUrl") {
         CHECK(out.error().kind == PreviewLookupError::Kind::Transient);
     }
 
+    TEST_CASE("Yugipedia clean-miss + YGOPRODeck transient is overall Transient") {
+        RoutingHttpClient http;
+        http.yugipediaBody = R"({"query":{"pages":{
+          "-1":{"title":"File:Whatever-LOB-EN-UR-UE.png","missing":""}
+        }}})";
+        http.ygoprodeckOk = false;
+
+        YuGiOhCardPreviewSource src{http};
+        const auto out = src.fetchImageUrl(
+            "No Such Card", "Legend of Blue Eyes White Dragon", "LOB-999||Ultra Rare||UE");
+        REQUIRE(out.isErr());
+        CHECK(out.error().kind == PreviewLookupError::Kind::Transient);
+    }
+
+    TEST_CASE("tuple-style setNo parsing trims fields and supports empty set code") {
+        FixedHttpClient http;
+        http.ok = true;
+        http.body = R"({"data":[{"name":"Dark Magician",
+          "card_images":[{"image_url":"https://images.ygoprodeck.com/std-dm.jpg"}]}]})";
+
+        YuGiOhCardPreviewSource src{http};
+        const auto out = src.fetchImageUrl(
+            "Dark Magician", "Legend of Blue Eyes White Dragon", "  || Ultra Rare || 1E ");
+        REQUIRE(out.isOk());
+        CHECK(out.value() == "https://images.ygoprodeck.com/std-dm.jpg");
+        CHECK(http.lastUrl.find("ygoprodeck.com") != std::string::npos);
+    }
+
     TEST_CASE("skips Yugipedia entirely when the set code is missing") {
         // Without a set code we can't construct any candidate filename - go
         // straight to the YGOPRODeck fallback to avoid wasting an HTTP call.
@@ -771,3 +841,36 @@ TEST_SUITE("YuGiOhCardPreviewSource::fetchImageUrl") {
         CHECK(http.lastUrl.find("yugipedia.com") == std::string::npos);
     }
 }
+
+TEST_SUITE("YuGiOhCardPreviewSource::detectFirstPrint") {
+    TEST_CASE("returns first variant from filtered request") {
+        FixedHttpClient http;
+        http.body = R"({
+          "data":[
+            {"name":"Dark Magician",
+             "card_sets":[
+               {"set_name":"Legend of Blue Eyes White Dragon","set_code":"LOB-005","set_rarity":"Ultra Rare"}
+             ]}
+          ]
+        })";
+        YuGiOhCardPreviewSource src{http};
+
+        const auto out = src.detectFirstPrint(
+            "Dark Magician", "Legend of Blue Eyes White Dragon");
+        REQUIRE(out.isOk());
+        CHECK(out.value().setNo == "LOB-005");
+        CHECK(out.value().rarity == "Ultra Rare");
+        CHECK(http.lastUrl.find("cardset=Legend%20of%20Blue%20Eyes%20White%20Dragon")
+              != std::string::npos);
+    }
+
+    TEST_CASE("propagates unfiltered fallback errors when both requests fail") {
+        FixedHttpClient http;
+        http.ok = false;
+        YuGiOhCardPreviewSource src{http};
+
+        const auto out = src.detectFirstPrint("Any", "Any Set");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "offline");
+    }
+}

tests/yugioh_set_source_tests.cpp

@@ -29,9 +29,18 @@ TEST_SUITE("YuGiOhSetSource::parseResponse") {
         ])";
         const auto out = YuGiOhSetSource::parseResponse(json);
         REQUIRE(out.isOk());
-        REQUIRE(out.value().size() == 2);
-        CHECK(out.value()[0].id == "AAA");
-        CHECK(out.value()[0].releaseDate == "2020/01/01");
+        bool foundA = false;
+        bool foundB = false;
+        for (const auto& set : out.value()) {
+            if (set.id == "AAA" && set.name == "Set A" && set.releaseDate == "2020/01/01") {
+                foundA = true;
+            }
+            if (set.id == "BBB" && set.name == "Set B" && set.releaseDate == "2021/02/03") {
+                foundB = true;
+            }
+        }
+        CHECK(foundA);
+        CHECK(foundB);
     }
 
     TEST_CASE("sorts by release date ascending") {
@@ -47,6 +56,86 @@ TEST_SUITE("YuGiOhSetSource::parseResponse") {
     TEST_CASE("missing array returns error") {
         CHECK(YuGiOhSetSource::parseResponse(R"({"data":[]})").isErr());
     }
+
+    TEST_CASE("adds 25th Anniversary aliases when upstream list misses them") {
+        const auto out = YuGiOhSetSource::parseResponse(R"([
+            {"set_name":"Legend of Blue Eyes White Dragon","set_code":"LOB","tcg_date":"2002-03-08"}
+        ])");
+        REQUIRE(out.isOk());
+
+        bool foundLob25th = false;
+        bool foundIoc25th = false;
+        for (const auto& set : out.value()) {
+            if (set.name == "Legend of Blue Eyes White Dragon (25th Anniversary Edition)"
+                && set.id == "LOB-25TH") {
+                foundLob25th = true;
+            }
+            if (set.name == "Invasion of Chaos (25th Anniversary Edition)" && set.id == "IOC-25TH") {
+                foundIoc25th = true;
+            }
+        }
+        CHECK(foundLob25th);
+        CHECK(foundIoc25th);
+    }
+
+    TEST_CASE("does not duplicate aliases that already exist by name") {
+        const auto out = YuGiOhSetSource::parseResponse(R"json([
+            {"set_name":"Legend of Blue Eyes White Dragon (25th Anniversary Edition)","set_code":"LOB-25TH","tcg_date":"2023-04-20"}
+        ])json");
+        REQUIRE(out.isOk());
+
+        int aliasCount = 0;
+        for (const auto& set : out.value()) {
+            if (set.name == "Legend of Blue Eyes White Dragon (25th Anniversary Edition)") {
+                ++aliasCount;
+            }
+        }
+        CHECK(aliasCount == 1);
+    }
+
+    TEST_CASE("malformed json returns parse error") {
+        const auto out = YuGiOhSetSource::parseResponse("{bad json");
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("YGOPRODeck set parse error:") != std::string::npos);
+    }
+
+    TEST_CASE("missing fields fall back to empty strings and keep parsing") {
+        const std::string json = R"([
+            {"set_name":"Set A"},
+            {"set_code":"BBB","tcg_date":"2021-02-03"}
+        ])";
+        const auto out = YuGiOhSetSource::parseResponse(json);
+        REQUIRE(out.isOk());
+        bool foundMissingCode = false;
+        bool foundMissingName = false;
+        for (const auto& set : out.value()) {
+            if (set.name == "Set A" && set.id.empty() && set.releaseDate.empty()) {
+                foundMissingCode = true;
+            }
+            if (set.id == "BBB" && set.name.empty() && set.releaseDate == "2021/02/03") {
+                foundMissingName = true;
+            }
+        }
+        CHECK(foundMissingCode);
+        CHECK(foundMissingName);
+    }
+
+    TEST_CASE("preserves slash-formatted dates and normalizes hyphen dates") {
+        const std::string json = R"([
+            {"set_name":"Slash Date","set_code":"S","tcg_date":"2024/01/01"},
+            {"set_name":"Hyphen Date","set_code":"H","tcg_date":"2024-01-02"}
+        ])";
+        const auto out = YuGiOhSetSource::parseResponse(json);
+        REQUIRE(out.isOk());
+        bool sawSlash = false;
+        bool sawHyphenNormalized = false;
+        for (const auto& set : out.value()) {
+            if (set.id == "S" && set.releaseDate == "2024/01/01") sawSlash = true;
+            if (set.id == "H" && set.releaseDate == "2024/01/02") sawHyphenNormalized = true;
+        }
+        CHECK(sawSlash);
+        CHECK(sawHyphenNormalized);
+    }
 }
 
 TEST_SUITE("YuGiOhSetSource::fetchAll") {
@@ -59,4 +148,13 @@ TEST_SUITE("YuGiOhSetSource::fetchAll") {
         CHECK(out.value().front().id == "X");
         CHECK(http.lastUrl == "https://db.ygoprodeck.com/api/v7/cardsets.php");
     }
+
+    TEST_CASE("network error is propagated") {
+        FixedHttpClient http;
+        http.ok = false;
+        YuGiOhSetSource src{http};
+        const auto out = src.fetchAll();
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "offline");
+    }
 }

ui_wx/include/ccm/ui/BaseCardEditDialog.hpp

@@ -352,11 +352,14 @@ private:
         failed.reserve(static_cast<std::size_t>(paths.size()));
 
         for (const auto& path : paths) {
+            const std::string setNameForImage = (game_ == Game::YuGiOh && !card_.set.id.empty())
+                                                    ? card_.set.id
+                                                    : card_.set.name;
             auto added = imageService_.addImage(game_,
                                                 std::filesystem::path(path.ToStdString()),
                                                 mode_ == EditMode::Create,
                                                 card_.id,
-                                                card_.set.name,
+                                                setNameForImage,
                                                 card_.name,
                                                 card_.images);
             if (!added) {

ui_wx/src/YuGiOhGameView.cpp

@@ -113,8 +113,11 @@ void YuGiOhGameView::onAddCard(wxWindow* parentWindow) {
 
     YuGiOhCard persisted = dlg.card();
     persisted.id = added.value();
+    const std::string setNameForImage = persisted.set.id.empty()
+                                            ? persisted.set.name
+                                            : persisted.set.id;
     auto normalized = images_.normalizeNamesForPersistedCard(
-        Game::YuGiOh, persisted.id, persisted.set.name, persisted.name, persisted.images);
+        Game::YuGiOh, persisted.id, setNameForImage, persisted.name, persisted.images);
     if (normalized) {
         if (normalized.value() != persisted.images) {
             persisted.images = std::move(normalized).value();