fix(filaments): guard against non-string profile names, log name collisions

Found during a targeted code review, not from a user report.

parse_profile()'s `name` field went straight through clean_name() without
routing through first_str() first, unlike filament_vendor/filament_type/
default_filament_colour right below it - all of which handle the
documented case where OrcaSlicer stores a field as ["value"] instead of a
plain string. If `data["name"]` was ever a list, clean_name()'s re.sub()
raised TypeError since it requires a string argument. Fixed by routing it
through first_str() like its neighbors.

Also added a debug log when sys_by_name (the system-profile lookup index)
overwrites an entry due to a name collision - clean_name() deliberately
collapses variant-suffixed profile names (e.g. "...@base" vs
"...@Anycubic Kobra X 0.4 nozzle") onto the same cleaned name, so a
collision is expected, but the resulting last-write-wins overwrite was
previously silent, making an unexpected inherits-parent resolution hard to
debug.

New tests in tests/test_orca_filaments_parser.py add the first dedicated
coverage for parse_profile()/parse_profile_bytes()/clean_name() - previous
tests only used pre-parsed profile dicts as fixtures and never exercised
the parsing logic itself.

orca_filaments.py

@@ -9,8 +9,11 @@ them as a normalized list with (id, name, vendor, type, color).
 from __future__ import annotations
 
 import json
+import logging
 import re
 
+log = logging.getLogger("kobrax.filaments")
+
 
 def first_str(value, default: str = "") -> str:
     """Orca profiles store some fields as ['value']. Returns the first
@@ -69,7 +72,17 @@ def parse_profile(data: dict, by_name: dict | None = None,
     if system_index:
         for p in system_index:
             if isinstance(p, dict) and p.get("name"):
-                sys_by_name[p["name"]] = p
+                pname = p["name"]
+                if pname in sys_by_name and sys_by_name[pname] is not p:
+                    # clean_name() deliberately collapses variant-suffixed
+                    # names (e.g. "...@base" vs "...@Anycubic Kobra X 0.4
+                    # nozzle") onto the same cleaned name - expected, but the
+                    # last-write-wins overwrite here was previously silent,
+                    # making an unexpected inherits-parent resolution hard to
+                    # debug.
+                    log.debug("orca_filaments: duplicate system profile name %r - "
+                              "overwriting %r with %r", pname, sys_by_name[pname].get("id"), p.get("id"))
+                sys_by_name[pname] = p
 
     def _resolve(key: str, depth: int = 5):
         cur_list = [data]
@@ -119,7 +132,7 @@ def parse_profile(data: dict, by_name: dict | None = None,
     if not fid or not isinstance(fid, str):
         return None
 
-    name_raw = data.get("name", fid)
+    name_raw = first_str(data.get("name"), fid)
     name = clean_name(name_raw)
     vendor = first_str(_resolve_full("filament_vendor")) or (path_vendor or "Generic")
     ftype  = first_str(_resolve_full("filament_type"), "")

tests/test_orca_filaments_parser.py

@@ -0,0 +1,182 @@
+"""orca_filaments.py parser robustness (code review finding).
+
+No dedicated test file existed for parse_profile()/parse_profile_bytes()/
+clean_name() before this - existing tests only used pre-parsed profile dicts
+as fixtures, never exercised the actual parsing logic.
+"""
+import json
+import logging
+
+from orca_filaments import clean_name, first_str, parse_profile, parse_profile_bytes
+
+
+def test_clean_name_strips_base_suffix():
+    assert clean_name("PolyTerra PLA @base") == "PolyTerra PLA"
+
+
+def test_clean_name_strips_printer_and_nozzle_suffix():
+    assert clean_name("Anycubic PLA @Anycubic Kobra X 0.4 nozzle") == "Anycubic PLA"
+
+
+def test_clean_name_strips_bare_nozzle_suffix():
+    assert clean_name("Anker Generic PLA 0.4 nozzle") == "Anker Generic PLA"
+
+
+def test_clean_name_returns_raw_when_stripping_leaves_nothing():
+    """An all-suffix name has nothing left after stripping - falls back to
+    the original raw string rather than returning an empty string."""
+    assert clean_name("@base") == "@base"
+
+
+def test_first_str_unwraps_single_element_list():
+    assert first_str(["PLA"]) == "PLA"
+
+
+def test_first_str_passes_through_plain_string():
+    assert first_str("PLA") == "PLA"
+
+
+def test_first_str_returns_default_for_empty_list():
+    assert first_str([], "fallback") == "fallback"
+
+
+def test_first_str_returns_default_for_other_types():
+    assert first_str(42, "fallback") == "fallback"
+    assert first_str(None, "fallback") == "fallback"
+
+
+def test_parse_profile_rejects_non_dict():
+    assert parse_profile([1, 2, 3]) is None
+    assert parse_profile("not a dict") is None
+    assert parse_profile(None) is None
+
+
+def test_parse_profile_rejects_stub_without_id_or_parent():
+    data = {"type": "filament"}  # no inherits, no filament_id
+    assert parse_profile(data) is None
+
+
+def test_parse_profile_rejects_instantiation_false():
+    data = {"type": "filament", "filament_id": "GFL01", "instantiation": "false"}
+    assert parse_profile(data) is None
+
+
+def test_parse_profile_minimal_valid_profile():
+    data = {
+        "type": "filament",
+        "filament_id": "GFL01",
+        "name": "Generic PLA",
+        "filament_vendor": ["Generic"],
+        "filament_type": ["PLA"],
+        "default_filament_colour": ["#FFFFFF"],
+    }
+    result = parse_profile(data)
+    assert result == {
+        "id": "GFL01",
+        "name": "Generic PLA",
+        "vendor": "Generic",
+        "type": "PLA",
+        "color": "#FFFFFF",
+    }
+
+
+def test_parse_profile_name_as_list_does_not_crash():
+    """Regression guard: `name` wasn't previously routed through first_str()
+    like the other fields are, unlike filament_vendor/filament_type/
+    default_filament_colour just below it - a list value here used to raise
+    TypeError inside clean_name()'s re.sub()."""
+    data = {
+        "type": "filament",
+        "filament_id": "GFL02",
+        "name": ["Geeetech PLA Basic"],
+        "filament_vendor": ["Geeetech"],
+        "filament_type": ["PLA"],
+    }
+    result = parse_profile(data)
+    assert result is not None
+    assert result["name"] == "Geeetech PLA Basic"
+
+
+def test_parse_profile_missing_name_falls_back_to_filament_id():
+    data = {"type": "filament", "filament_id": "GFL03", "filament_vendor": ["Generic"]}
+    result = parse_profile(data)
+    assert result["name"] == "GFL03"
+
+
+def test_parse_profile_missing_optional_fields_default_to_empty_string():
+    data = {"type": "filament", "filament_id": "GFL04", "name": "Mystery Filament"}
+    result = parse_profile(data)
+    assert result["type"] == ""
+    assert result["color"] == ""
+    assert result["vendor"] == "Generic"  # no path_vendor given either
+
+
+def test_parse_profile_inherits_via_by_name():
+    parent = {"type": "filament", "filament_id": "GFL05", "filament_vendor": ["Geeetech"], "filament_type": ["PLA"]}
+    child = {"type": "filament", "inherits": "Geeetech PLA @base", "name": "Geeetech PLA Basic"}
+    by_name = {"Geeetech PLA @base": [parent]}
+    result = parse_profile(child, by_name=by_name)
+    assert result is not None
+    assert result["id"] == "GFL05"
+    assert result["vendor"] == "Geeetech"
+    assert result["type"] == "PLA"
+
+
+def test_parse_profile_inherits_via_system_index():
+    system_index = [{
+        "id": "GFL06", "name": "Geeetech PLA", "vendor": "Geeetech", "type": "PLA", "color": "",
+    }]
+    user_profile = {"type": "filament", "inherits": "Geeetech PLA @base", "name": "My Geeetech Override"}
+    result = parse_profile(user_profile, system_index=system_index)
+    assert result is not None
+    assert result["id"] == "GFL06"
+    assert result["vendor"] == "Geeetech"
+
+
+def test_parse_profile_inherits_cycle_does_not_infinite_loop():
+    """A inherits B, B inherits A - _resolve()'s hard depth=5 bound must
+    terminate this rather than recursing forever."""
+    a = {"type": "filament", "inherits": "B"}
+    b = {"type": "filament", "inherits": "A"}
+    by_name = {"A": [a], "B": [b]}
+    # Neither profile has a filament_id anywhere in the cycle - must return
+    # None (not hang, not crash) after exhausting the depth limit.
+    result = parse_profile(a, by_name=by_name)
+    assert result is None
+
+
+def test_parse_profile_duplicate_system_names_logs_and_uses_last(caplog):
+    """clean_name() deliberately collapses variant-suffixed names onto the
+    same cleaned name - sys_by_name's last-write-wins overwrite on collision
+    is expected, but must now be observable via a debug log instead of
+    silent."""
+    system_index = [
+        {"id": "GFL07", "name": "PolyTerra PLA", "vendor": "Polymaker", "type": "PLA", "color": ""},
+        {"id": "GFL08", "name": "PolyTerra PLA", "vendor": "Polymaker", "type": "PLA", "color": ""},
+    ]
+    user_profile = {"type": "filament", "inherits": "PolyTerra PLA @base", "name": "Override"}
+    with caplog.at_level(logging.DEBUG, logger="kobrax.filaments"):
+        result = parse_profile(user_profile, system_index=system_index)
+    assert result is not None
+    assert result["id"] == "GFL08"  # last one wins, as before
+    assert any("duplicate system profile name" in r.message for r in caplog.records)
+
+
+def test_parse_profile_bytes_valid_json():
+    blob = json.dumps({
+        "type": "filament", "filament_id": "GFL09", "name": "Test PLA",
+        "filament_vendor": ["Test"], "filament_type": ["PLA"],
+    }).encode("utf-8")
+    result = parse_profile_bytes(blob)
+    assert result is not None
+    assert result["id"] == "GFL09"
+
+
+def test_parse_profile_bytes_malformed_json_returns_none():
+    assert parse_profile_bytes(b"{not valid json") is None
+
+
+def test_parse_profile_bytes_non_dict_json_returns_none():
+    assert parse_profile_bytes(b"[1, 2, 3]") is None
+    assert parse_profile_bytes(b'"just a string"') is None
+    assert parse_profile_bytes(b"42") is None