fix(spoolman): persist color_name edits and stop form round-tripping the subtype synth fallback (#1319)

  Editing a spool's color name on Spoolman-backed inventory appeared to
  accept the new value but the inventory list column and the next edit
  showed it back to the subtype. Three layers stacked to produce this:

  1. find_or_create_filament matches by material/name/color_hex/vendor —
     color_name is intentionally not part of the match key, but on a
     match it returned the existing filament's id unchanged, silently
     dropping the new value.
  2. The read helper falls back to subtype when filament.color_name is
     empty (kept on purpose: without it Spoolman installs that don't
     fill the field render every spool as "Unknown color").
  3. The edit form prefilled color_name from spool.color_name — which
     on those installs was the synth value. Changing subtype but not
     color_name silently round-tripped the OLD subtype back to Spoolman
     as if it were a real user-set color_name.

  Fixes:
  - find_or_create_filament now patches the matched filament's
    color_name via the existing patch_filament wrapper when the request
    differs. Parameter convention: None = don't touch, "" = explicit
    clear, any other string = set/update. A patch failure is logged but
    does not block the match.
  - The PATCH route uses model_fields_set to distinguish "field omitted"
    from "field explicitly set to null" (mirrors the existing
    storage_location pattern at the same site).
  - The map helper returns color_name_is_synthesized: bool. The edit
    form leaves the input blank when true, so the user sees the real
    stored state and can't accidentally round-trip the synth value back.

backend/app/api/routes/_spoolman_helpers.py

@@ -26,6 +26,7 @@ class MappedSpoolFields(TypedDict):
     subtype: str | None
     brand: str | None
     color_name: str | None
+    color_name_is_synthesized: bool
     rgba: str | None
     label_weight: int | None
     core_weight: int | None
@@ -276,7 +277,12 @@ def _map_spoolman_spool(spool: dict) -> MappedSpoolFields:
     # prefix (the same string the `subtype` field already carries — typically
     # "Basic Red" / "PLA+ Black" / etc.) so the user can tell spools apart at
     # a glance even on Spoolman installs that don't fill color_name.
-    color_name: str | None = filament.get("color_name") or subtype or None
+    # color_name_is_synthesized: surfaced so the edit form can avoid prefilling
+    # the synth value back into the input, which would otherwise round-trip the
+    # subtype string as if it were a user-set color_name (#1319).
+    stored_color_name = filament.get("color_name") or None
+    color_name: str | None = stored_color_name or subtype or None
+    color_name_is_synthesized: bool = stored_color_name is None and color_name is not None
 
     nozzle_temp_raw = filament.get("settings_extruder_temp")
     nozzle_temp_min: int | None = _safe_int(nozzle_temp_raw, 0) or None
@@ -286,6 +292,7 @@ def _map_spoolman_spool(spool: dict) -> MappedSpoolFields:
         "material": material,
         "subtype": subtype,
         "color_name": color_name,
+        "color_name_is_synthesized": color_name_is_synthesized,
         "rgba": rgba,
         "brand": vendor.get("name") or None,
         "label_weight": label_weight,

backend/app/api/routes/spoolman_inventory.py

@@ -564,7 +564,13 @@ async def update_spool(
     material = data.material if data.material is not None else cur_mat
     subtype = data.subtype if data.subtype is not None else cur_subtype
     brand = data.brand if data.brand is not None else (cur_vendor.get("name") or None)
-    color_name = data.color_name if data.color_name is not None else (cur_filament.get("color_name") or None)
+    # color_name uses model_fields_set so explicit null (clear) is distinguishable
+    # from "field omitted" (don't touch). find_or_create_filament's convention:
+    # None = don't touch, "" = explicit clear, "value" = set.
+    if "color_name" in data.model_fields_set:
+        color_name = data.color_name if data.color_name is not None else ""
+    else:
+        color_name = cur_filament.get("color_name") or None
     cur_color = (cur_filament.get("color_hex") or "808080").upper().removeprefix("#")
     rgba = data.rgba if data.rgba is not None else (cur_color + "FF")
     label_weight = data.label_weight if data.label_weight is not None else int(cur_filament.get("weight") or 1000)

backend/app/services/spoolman.py

@@ -637,6 +637,26 @@ class SpoolmanClient:
             vendor_match = (not brand) or f_vendor_name == (brand or "").strip().lower()
 
             if material_match and name_match and color_match and vendor_match:
+                # #1319: color_name is not part of the match key, but if the
+                # caller passed a value that differs from what's stored, update
+                # the filament — otherwise the user's edit is silently dropped
+                # and the inventory read falls back to subtype, making it look
+                # like color_name "reverts" to the subtype on every save.
+                # Convention: None = "don't touch"; "" = explicit clear; any
+                # other string = set/update.
+                if color_name is not None:
+                    existing = (f.get("color_name") or "").strip()
+                    requested = color_name.strip()
+                    if requested != existing:
+                        payload_value: str | None = requested if requested else None
+                        try:
+                            await self.patch_filament(f["id"], {"color_name": payload_value})
+                        except Exception as e:
+                            logger.warning(
+                                "Failed to update color_name on filament %s: %s",
+                                f["id"],
+                                e,
+                            )
                 return f["id"]
 
         filament = await self.create_filament(

backend/tests/integration/test_spoolman_inventory_api.py

@@ -321,6 +321,42 @@ class TestSpoolmanInventoryCRUD:
         assert response.status_code == 200
         mock_spoolman_client.update_spool_full.assert_called_once()
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_update_with_explicit_null_color_name_clears(
+        self,
+        async_client: AsyncClient,
+        spoolman_settings,
+        mock_spoolman_client,
+    ):
+        """#1319 follow-up: explicit color_name=null in the PATCH body means
+        "clear" — route translates it to "" so find_or_create_filament patches
+        the matched filament with color_name=None."""
+        payload = {"color_name": None}
+        response = await async_client.patch("/api/v1/spoolman/inventory/spools/42", json=payload)
+        assert response.status_code == 200
+        mock_spoolman_client.find_or_create_filament.assert_called_once()
+        kwargs = mock_spoolman_client.find_or_create_filament.call_args.kwargs
+        assert kwargs["color_name"] == ""
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_update_without_color_name_keeps_current(
+        self,
+        async_client: AsyncClient,
+        spoolman_settings,
+        mock_spoolman_client,
+    ):
+        """#1319 follow-up: when color_name is omitted from the PATCH body the
+        current value is kept — None passed to find_or_create_filament means
+        "don't touch"."""
+        payload = {"note": "only updating note"}
+        response = await async_client.patch("/api/v1/spoolman/inventory/spools/42", json=payload)
+        assert response.status_code == 200
+        kwargs = mock_spoolman_client.find_or_create_filament.call_args.kwargs
+        # SAMPLE_SPOOLMAN_SPOOL has no color_name field, so cur fallback is None.
+        assert kwargs["color_name"] is None
+
     @pytest.mark.asyncio
     @pytest.mark.integration
     async def test_update_spool_not_found(

backend/tests/unit/test_spoolman_inventory_helpers.py

@@ -179,6 +179,23 @@ class TestMapSpoolmanSpool:
         }
         result = _map_spoolman_spool(spool)
         assert result["color_name"] == "Sunrise Orange"
+        # Real stored value — not synthesised from subtype.
+        assert result["color_name_is_synthesized"] is False
+
+    def test_color_name_flags_synthesized_when_falling_back_to_subtype(self):
+        """#1319: when the read falls back to subtype, the response must flag it
+        so the edit form doesn't round-trip the synth value back to Spoolman."""
+        spool = {
+            **MINIMAL_SPOOL,
+            "filament": {
+                **MINIMAL_SPOOL["filament"],
+                "name": "PLA Basic Red",
+                # No color_name field.
+            },
+        }
+        result = _map_spoolman_spool(spool)
+        assert result["color_name"] == "Basic Red"
+        assert result["color_name_is_synthesized"] is True
 
     def test_color_name_falls_back_to_subtype_when_field_missing(self):
         """Spoolman doesn't standardise color_name; the LinkSpoolModal would
@@ -216,6 +233,8 @@ class TestMapSpoolmanSpool:
         result = _map_spoolman_spool(spool)
         assert result["subtype"] is None
         assert result["color_name"] is None
+        # No synth happened — nothing to fall back to.
+        assert result["color_name_is_synthesized"] is False
 
     def test_color_hex_with_hash_prefix_stripped(self):
         spool = {**MINIMAL_SPOOL, "filament": {**MINIMAL_SPOOL["filament"], "color_hex": "#00FF00"}}

backend/tests/unit/test_spoolman_inventory_methods.py

@@ -322,6 +322,81 @@ class TestFindOrCreateFilament:
             result = await client.find_or_create_filament("PLA", "Basic", "Bambu Lab", "FF0000", 1000)
         assert result == 7
 
+    @pytest.mark.asyncio
+    async def test_patches_color_name_on_existing_filament_when_changed(self, client):
+        """#1319: color_name is not part of the match key, so when a caller
+        updates a spool with a new color_name and the material/name/color/vendor
+        still match an existing filament, the existing filament's color_name
+        must be patched — otherwise the user's edit is silently dropped."""
+        existing = {**SAMPLE_FILAMENT, "color_name": None}
+        with (
+            patch.object(client, "find_or_create_vendor", AsyncMock(return_value=3)),
+            patch.object(client, "get_filaments", AsyncMock(return_value=[existing])),
+            patch.object(client, "patch_filament", AsyncMock(return_value={"id": 7})) as mock_patch,
+        ):
+            result = await client.find_or_create_filament(
+                "PLA", "Basic", "Bambu Lab", "FF0000", 1000, color_name="Sunny Yellow"
+            )
+        assert result == 7
+        mock_patch.assert_called_once_with(7, {"color_name": "Sunny Yellow"})
+
+    @pytest.mark.asyncio
+    async def test_does_not_patch_when_color_name_unchanged(self, client):
+        existing = {**SAMPLE_FILAMENT, "color_name": "Sunny Yellow"}
+        with (
+            patch.object(client, "find_or_create_vendor", AsyncMock(return_value=3)),
+            patch.object(client, "get_filaments", AsyncMock(return_value=[existing])),
+            patch.object(client, "patch_filament", AsyncMock()) as mock_patch,
+        ):
+            result = await client.find_or_create_filament(
+                "PLA", "Basic", "Bambu Lab", "FF0000", 1000, color_name="Sunny Yellow"
+            )
+        assert result == 7
+        mock_patch.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_does_not_patch_when_color_name_empty(self, client):
+        """An empty/None color_name should not clobber an existing value."""
+        existing = {**SAMPLE_FILAMENT, "color_name": "Sunny Yellow"}
+        with (
+            patch.object(client, "find_or_create_vendor", AsyncMock(return_value=3)),
+            patch.object(client, "get_filaments", AsyncMock(return_value=[existing])),
+            patch.object(client, "patch_filament", AsyncMock()) as mock_patch,
+        ):
+            result = await client.find_or_create_filament("PLA", "Basic", "Bambu Lab", "FF0000", 1000, color_name=None)
+        assert result == 7
+        mock_patch.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_clears_color_name_when_empty_string_passed(self, client):
+        """#1319 follow-up: empty string means "explicit clear" — the route
+        layer translates a wire-level null into "" so the user can blank the
+        field on a previously-set spool."""
+        existing = {**SAMPLE_FILAMENT, "color_name": "Sunny Yellow"}
+        with (
+            patch.object(client, "find_or_create_vendor", AsyncMock(return_value=3)),
+            patch.object(client, "get_filaments", AsyncMock(return_value=[existing])),
+            patch.object(client, "patch_filament", AsyncMock(return_value={"id": 7})) as mock_patch,
+        ):
+            result = await client.find_or_create_filament("PLA", "Basic", "Bambu Lab", "FF0000", 1000, color_name="")
+        assert result == 7
+        mock_patch.assert_called_once_with(7, {"color_name": None})
+
+    @pytest.mark.asyncio
+    async def test_patch_failure_does_not_block_match(self, client):
+        """A patch_filament failure must not prevent returning the matched id —
+        save should still link the spool to the correct filament."""
+        existing = {**SAMPLE_FILAMENT, "color_name": None}
+        with (
+            patch.object(client, "find_or_create_vendor", AsyncMock(return_value=3)),
+            patch.object(client, "get_filaments", AsyncMock(return_value=[existing])),
+            patch.object(client, "patch_filament", AsyncMock(side_effect=SpoolmanUnavailableError("boom"))),
+        ):
+            result = await client.find_or_create_filament(
+                "PLA", "Basic", "Bambu Lab", "FF0000", 1000, color_name="Sunny Yellow"
+            )
+        assert result == 7
+
     @pytest.mark.asyncio
     async def test_creates_filament_when_no_match(self, client):
         new_filament = {"id": 99, "name": "PETG Pro"}

frontend/src/api/client.ts

@@ -2345,6 +2345,11 @@ export interface InventorySpool {
   material: string;
   subtype: string | null;
   color_name: string | null;
+  // True when color_name was synthesised from subtype because Spoolman has no
+  // stored value (Spoolman-backed inventory only). The edit form uses this to
+  // leave the input blank, so the user doesn't round-trip the synth value
+  // back to Spoolman as if it were a real user-set color_name (#1319).
+  color_name_is_synthesized?: boolean;
   rgba: string | null;
   // Multi-colour gradient stops (#1154): comma-separated 6/8-char hex.
   extra_colors: string | null;

frontend/src/components/SpoolFormModal.tsx

@@ -303,7 +303,11 @@ export function SpoolFormModal({
           material: spool.material || '',
           subtype: spool.subtype || '',
           brand: spool.brand || '',
-          color_name: spool.color_name || '',
+          // #1319: leave color_name blank when the backend reports it was
+          // synthesised from subtype — otherwise the form would round-trip
+          // the synth value to Spoolman on save as if the user had set it,
+          // which is what produced the "color reverts to subtype" symptom.
+          color_name: spool.color_name_is_synthesized ? '' : (spool.color_name || ''),
           rgba: validRgba,
           extra_colors: spool.extra_colors || '',
           effect_type: spool.effect_type || '',

static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-BfJWmysU.js"></script>
+    <script type="module" crossorigin src="/assets/index-D5ddq2Sh.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-BkYu3kLs.css">
   </head>
   <body>