From ae36d3ac13ec1ad4d8f7ed80ed94427c6d3173ca Mon Sep 17 00:00:00 2001 From: jmoore-skild Date: Sat, 1 Aug 2026 23:06:59 -0400 Subject: [PATCH] fix(backup): match a K-profile on its own extruder, not just its filament (#2656) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_match_kprofile` scoped candidates by `filament_id` alone, and `_current_kprofile_index` reads the live index per nozzle *diameter* — so on a dual-nozzle printer both extruders' profiles come back in one list. On an H2D with the same filament calibrated on both extruders, the `setting_id` arm then matched whichever profile the printer happened to list first. A backed-up extruder-0 entry took extruder 1's slot and went into the batch as `{extruder_id: 0, cali_idx: }`, writing one extruder's calibration over the other's and counting it restored. With an entry per extruder — the ordinary case, since the same preset on both nozzles is what a dual-nozzle printer is for — the two swapped slots and clobbered each other. The `name` arm and the single-candidate fallback were equally unscoped, so this was never only about the ambiguous case. The data was already in hand and already being read: the backup entry carries `extruder_id` (`:1546` copies it straight into the outgoing dict) and `KProfile` has carried `extruder_id: int` on the live side all along. Scope `candidates` by it the same way `filament_id` already scopes them, and the single-candidate fallback narrows with them — ambiguity is judged within one extruder now. Conditional on both sides saying which extruder they mean. A pre-#2656 backup has no `extruder_id`, and a live index that reports none must not turn every entry into an add — that would be a far worse regression than the bug. Two controls cover each direction of that. `claimed` (G3) is untouched: it is per nozzle-loop and this only narrows the candidate set feeding it. Tests: +4 across the three restore files (270 -> 274). Fail-pre-fix 2 (each extruder keeps its own slot; the other extruder's profile is not a stand-in), controls that pass either way 2. --- backend/app/services/github_restore.py | 20 +++- backend/tests/unit/test_github_restore.py | 109 +++++++++++++++++++++- 2 files changed, 125 insertions(+), 4 deletions(-) diff --git a/backend/app/services/github_restore.py b/backend/app/services/github_restore.py index 97b0ae5c0..9215fc8fd 100644 --- a/backend/app/services/github_restore.py +++ b/backend/app/services/github_restore.py @@ -1642,7 +1642,9 @@ class GitHubRestoreService: is the strongest signal; a delete-then-add edit regenerates it, so fall back to the display name, which Bambuddy's own editor preserves. Both are scoped by ``filament_id`` — the same preset on a different - filament is a different profile. + filament is a different profile — and by ``extruder_id``, because on a + dual-nozzle printer the same preset on the other extruder is a different + profile too. ``claimed`` holds the slot ids already taken by earlier entries in this nozzle's loop, and no live profile may be claimed twice. Without it, two @@ -1658,6 +1660,22 @@ class GitHubRestoreService: return None candidates = [c for c in current if c.filament_id == filament_id] + + # The live index is read per nozzle *diameter*, so on an H2D both + # extruders' profiles come back together. With the same filament + # calibrated on both — the ordinary case on a dual-nozzle printer, not an + # exotic one — filament_id alone lets extruder 0's backed-up entry match + # extruder 1's live profile, and the batch then carries + # {extruder_id: 0, cali_idx: }: one extruder's + # calibration written over the other's, counted restored. + # + # Conditional on both sides saying which extruder they mean. A pre-#2656 + # backup carries no extruder_id, and a live index that reports none must + # not turn every entry into an add. + extruder_id = entry.get("extruder_id") + if isinstance(extruder_id, int) and any(getattr(c, "extruder_id", None) is not None for c in candidates): + candidates = [c for c in candidates if getattr(c, "extruder_id", None) == extruder_id] + available = [c for c in candidates if c.slot_id not in claimed] if not available: return None diff --git a/backend/tests/unit/test_github_restore.py b/backend/tests/unit/test_github_restore.py index 52fd8e2fe..91b5e5dc7 100644 --- a/backend/tests/unit/test_github_restore.py +++ b/backend/tests/unit/test_github_restore.py @@ -1232,9 +1232,19 @@ class TestRestoreArchives: class TestRestoreKprofiles: @staticmethod - def _live(slot_id, filament_id="GFA00", name="Bambu PLA", setting_id="PFUS123"): - """One profile as the printer currently reports it.""" - return SimpleNamespace(slot_id=slot_id, filament_id=filament_id, name=name, setting_id=setting_id) + def _live(slot_id, filament_id="GFA00", name="Bambu PLA", setting_id="PFUS123", extruder_id=0): + """One profile as the printer currently reports it. + + ``extruder_id`` mirrors ``KProfile`` (bambu_mqtt.py), which has carried + it all along; single-nozzle printers report 0. + """ + return SimpleNamespace( + slot_id=slot_id, + filament_id=filament_id, + name=name, + setting_id=setting_id, + extruder_id=extruder_id, + ) def _client(self, live=None, sent="7", ack=(True, "")): """A connected printer client. @@ -1527,6 +1537,99 @@ class TestRestoreKprofiles: profiles, _ = client.set_kprofiles_batch.call_args.args assert [p["cali_idx"] for p in profiles] == [1, -1] + # --- the match is scoped to the extruder it was calibrated on ----------- + # + # get_kprofiles reads per nozzle *diameter*, so on a dual-nozzle printer + # both extruders come back in one list. Scoping candidates on filament_id + # alone let one extruder's calibration be written over the other's. + + @pytest.mark.asyncio + async def test_each_extruders_profile_lands_on_its_own_extruder(self, db_session, printer_factory): + """The same preset calibrated on both extruders of an H2D. + + Both live profiles share a filament_id *and* a setting_id, so the + setting_id arm matched whichever the printer happened to list first — + and with an entry per extruder the two swapped slots, each overwriting + the other's calibration while the tally counted both restored. + """ + await printer_factory(serial_number="00M09A123456789") + payload = self._payload() + entries = payload["kprofiles/00M09A123456789/0.4.json"]["profiles"] + entries.append({**entries[0], "extruder_id": 1, "nozzle_id": "HS00-0.4-R"}) + client = self._client( + live=[ + # Right extruder first, which is what made the bug bite. + self._live(slot_id=1001, extruder_id=1), + self._live(slot_id=1000, extruder_id=0), + ] + ) + tally = _CategoryTally() + + with patch("backend.app.services.github_restore.printer_manager") as manager: + manager.get_client = MagicMock(return_value=client) + await _service()._restore_kprofiles(db_session, payload, tally) + + profiles, _ = client.set_kprofiles_batch.call_args.args + assert [(p["extruder_id"], p["cali_idx"]) for p in profiles] == [(0, 1000), (1, 1001)] + assert tally.restored == 2 + assert not any("added as new profiles" in note for note in _messages(tally)) + + @pytest.mark.asyncio + async def test_the_other_extruders_profile_is_not_a_candidate(self, db_session, printer_factory): + """One backed-up entry, and the only live profile is the other extruder's. + + Adding as new is the right answer: extruder 0's calibration is not a + stand-in for extruder 1's, however well the filament and preset line up. + """ + await printer_factory(serial_number="00M09A123456789") + client = self._client(live=[self._live(slot_id=1001, extruder_id=1)]) + tally = _CategoryTally() + + with patch("backend.app.services.github_restore.printer_manager") as manager: + manager.get_client = MagicMock(return_value=client) + await _service()._restore_kprofiles(db_session, self._payload(), tally) + + profiles, _ = client.set_kprofiles_batch.call_args.args + assert profiles[0]["cali_idx"] == -1 + assert any("added as new profiles" in note for note in _messages(tally)) + + @pytest.mark.asyncio + async def test_an_entry_without_an_extruder_id_still_matches(self, db_session, printer_factory): + """Control: a pre-#2656 backup carries no extruder_id. + + A missing key must leave the match exactly as it was, not turn every + entry into an add. + """ + await printer_factory(serial_number="00M09A123456789") + payload = self._payload() + payload["kprofiles/00M09A123456789/0.4.json"]["profiles"][0].pop("extruder_id") + client = self._client(live=[self._live(slot_id=4606)]) + tally = _CategoryTally() + + with patch("backend.app.services.github_restore.printer_manager") as manager: + manager.get_client = MagicMock(return_value=client) + await _service()._restore_kprofiles(db_session, payload, tally) + + profiles, _ = client.set_kprofiles_batch.call_args.args + assert profiles[0]["cali_idx"] == 4606 + assert tally.restored == 1 + + @pytest.mark.asyncio + async def test_a_live_index_that_reports_no_extruder_still_matches(self, db_session, printer_factory): + """Control: the same, for a printer whose profiles carry no extruder_id.""" + await printer_factory(serial_number="00M09A123456789") + live = SimpleNamespace(slot_id=4606, filament_id="GFA00", name="Bambu PLA", setting_id="PFUS123") + client = self._client(live=[live]) + tally = _CategoryTally() + + with patch("backend.app.services.github_restore.printer_manager") as manager: + manager.get_client = MagicMock(return_value=client) + await _service()._restore_kprofiles(db_session, self._payload(), tally) + + profiles, _ = client.set_kprofiles_batch.call_args.args + assert profiles[0]["cali_idx"] == 4606 + assert tally.restored == 1 + @pytest.mark.asyncio async def test_unknown_serial_is_skipped_with_reason(self, db_session): tally = _CategoryTally()