From 13c37ffe5130c1acc364e9f330f822ae7a694a76 Mon Sep 17 00:00:00 2001 From: Sergey Dontsov Date: Fri, 31 Jul 2026 11:08:51 +0300 Subject: [PATCH] fix(vp): scope saved AMS mapping to the printer it was resolved against MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review fixes for #2700. Blocking: the toggle didn't actually gate the archive write. archive.py's promotion fired for any print_data carrying ams_mapping, but bambu_mqtt's request-topic interception captures ams_mapping unconditionally for every print source (slicer-direct LAN prints included). Since main.py's real-printer auto-archive path forwards the full MQTT payload as print_data, every archive on any install — VP or not — grew extra_data.slicer_ams_mapping. Fixed by replacing the print_data-sniffing with an explicit `slicer_ams_mapping` param on archive_print() that only the VP-queue path (already gated on save_ams_mapping) ever passes. Blocking: a saved mapping could get reused on a printer it was never resolved against — tray IDs only mean something relative to one printer's AMS layout. extra_data.slicer_ams_mapping is now stored as {mapping, printer_id} instead of a bare array: - add_to_queue's fallback only fires when the reprint's target printer_id matches the mapping's origin printer. - The frontend's archiveAmsMapping only surfaces (and the Mapping button only appears) when the print modal's selected printer matches too. - A model-based VP (target_printer_id=None, no MQTT bridge to any real printer) never stamps a mapping in the first place — there's no live AMS layout for the slicer to have resolved tray IDs against. Also from review: - Multi-plate archives now get the Mapping button too (the per-plate FilamentMapping loop was missing archiveAmsMapping entirely). - Added coverage for the previously-untested late-MQTT archive patch path (_restamp_recent_queue_item), including the model-based-VP skip case. - usingArchiveMapping now also resets on printer change, not just plate/archive (it already worked via the printer-scoping above, but is now an explicit dependency too). - The Mapping button's revert (OFF) now undoes only the slots it itself set, not every manual pick in scope — matches the comment above it. - Added a comment on why negative-value slots (external spool) are skipped rather than cleared when applying a saved mapping. --- backend/app/api/routes/print_queue.py | 30 +- backend/app/services/archive.py | 48 ++- .../app/services/virtual_printer/manager.py | 51 +++- .../tests/integration/test_print_queue_api.py | 65 +++- .../unit/services/test_virtual_printer.py | 281 +++++++++++++++++- .../components/PrintModal/FilamentMapping.tsx | 26 +- frontend/src/components/PrintModal/index.tsx | 18 +- frontend/src/pages/ArchivesPage.tsx | 8 +- 8 files changed, 468 insertions(+), 59 deletions(-) diff --git a/backend/app/api/routes/print_queue.py b/backend/app/api/routes/print_queue.py index d55f83837..450cd3de8 100644 --- a/backend/app/api/routes/print_queue.py +++ b/backend/app/api/routes/print_queue.py @@ -657,15 +657,27 @@ async def add_to_queue( # to the exact same physical spool instead of the scheduler re-deriving a # (possibly ambiguous) mapping from just the file's static type/color. # - # Note this is unconditional — it applies regardless of whether the - # physical spool in that slot has changed since the original print. #1308 - # covers re-verifying a stored mapping against live AMS state at dispatch - # time; that check is a separate PR and, once merged, will also catch a - # stale slot inherited through this fallback. - if ams_mapping_json is None and archive and archive.extra_data: - saved_mapping = archive.extra_data.get("slicer_ams_mapping") - if isinstance(saved_mapping, list) and saved_mapping: - ams_mapping_json = json.dumps(saved_mapping) + # Global tray IDs only mean something relative to the specific printer + # they were resolved against, so this only fires when the reprint targets + # that exact printer (`extra_data.slicer_ams_mapping.printer_id`) — never + # for a model-based dispatch (data.printer_id is None) or a reprint aimed + # at a different printer, where the same tray number can hold a + # completely different spool (#2700 review). + # + # Note this is otherwise unconditional — it applies regardless of whether + # the physical spool in that slot has changed since the original print. + # #1308 covers re-verifying a stored mapping against live AMS state at + # dispatch time; that check is a separate PR and, once merged, will also + # catch a stale slot inherited through this fallback. + if ams_mapping_json is None and archive and archive.extra_data and data.printer_id is not None: + saved = archive.extra_data.get("slicer_ams_mapping") + if ( + isinstance(saved, dict) + and saved.get("printer_id") == data.printer_id + and isinstance(saved.get("mapping"), list) + and saved["mapping"] + ): + ams_mapping_json = json.dumps(saved["mapping"]) items = [] for i in range(quantity): item = PrintQueueItem( diff --git a/backend/app/services/archive.py b/backend/app/services/archive.py index 1f79759aa..51e5d7a45 100644 --- a/backend/app/services/archive.py +++ b/backend/app/services/archive.py @@ -1144,6 +1144,8 @@ class ArchiveService: prefer_filename_for_name: bool = False, plate_id: int | None = None, library_file_id: int | None = None, + slicer_ams_mapping: list[int] | None = None, + slicer_ams_mapping_printer_id: int | None = None, ) -> PrintArchive | None: """Archive a 3MF file with metadata. @@ -1166,6 +1168,21 @@ class ArchiveService: metadata. Used by virtual-printer flows so users who rename a job in BambuStudio's "send to printer" dialog see that name instead of the creator-baked title (#1152). + slicer_ams_mapping: The slicer's own live-resolved AMS-slot pick, to persist + onto `extra_data.slicer_ams_mapping` for a later reprint to reuse. Deliberately + a distinct parameter, not read off `print_data["ams_mapping"]` — that key is + populated on every MQTT print-start callback regardless of source (bambu_mqtt's + request-topic interception captures it for slicer-direct LAN prints too), so + promoting it unconditionally would stamp every archive on installs with no + virtual printer at all. Callers that gate this behind an opt-in (the VP-queue + "Save AMS mapping" toggle) pass it explicitly; everyone else leaves it unset. + slicer_ams_mapping_printer_id: The printer `slicer_ams_mapping`'s tray IDs were + resolved against. Required alongside `slicer_ams_mapping` — a global tray ID + only means something relative to one printer's specific AMS layout, so a + mapping saved without knowing which printer it came from can't be safely + reused later on any printer, including the same one (there'd be no way to + tell). A model-based VP with no fixed target printer has no valid value to + pass here and must leave both params unset. """ # Verify printer exists if specified if printer_id is not None: @@ -1253,18 +1270,25 @@ class ArchiveService: # Merge with print data from MQTT if print_data: metadata["_print_data"] = print_data - # Promote the slicer's own live-resolved AMS-slot pick (captured - # from the project_file MQTT command by the VP-queue path — see - # `_extract_slicer_ams_mapping_json` in virtual_printer/manager.py) - # to a stable top-level extra_data key. Lets a later reprint reuse - # the exact tray the user picked/BambuStudio auto-matched at slice - # time instead of the scheduler re-deriving one from just the - # file's static type/color, which can land on the wrong physical - # spool when that match isn't unique. Top-level (not nested under - # the `_print_data` diagnostic bag) so API consumers have a single - # stable path: `archive.extra_data.slicer_ams_mapping`. - if print_data.get("ams_mapping"): - metadata["slicer_ams_mapping"] = print_data["ams_mapping"] + + # Promote the slicer's own live-resolved AMS-slot pick, when the caller + # explicitly opted in (see the `slicer_ams_mapping` param docstring for + # why this is NOT read off `print_data["ams_mapping"]`), to a stable + # top-level extra_data key. Lets a later reprint reuse the exact tray + # the user picked/BambuStudio auto-matched at slice time instead of the + # scheduler re-deriving one from just the file's static type/color, + # which can land on the wrong physical spool when that match isn't + # unique. Top-level (not nested under the `_print_data` diagnostic bag) + # so API consumers have a single stable path: + # `archive.extra_data.slicer_ams_mapping`. Stored together with the + # printer it was resolved against — see `slicer_ams_mapping_printer_id` + # param docstring — so a later reprint can tell whether it's even + # applicable before trying to reuse it. + if slicer_ams_mapping and slicer_ams_mapping_printer_id is not None: + metadata["slicer_ams_mapping"] = { + "mapping": slicer_ams_mapping, + "printer_id": slicer_ams_mapping_printer_id, + } # Determine status and timestamps status = print_data.get("status", "completed") if print_data else "archived" diff --git a/backend/app/services/virtual_printer/manager.py b/backend/app/services/virtual_printer/manager.py index 09c666025..67ff37aa2 100644 --- a/backend/app/services/virtual_printer/manager.py +++ b/backend/app/services/virtual_printer/manager.py @@ -521,7 +521,15 @@ class VirtualPrinterInstance: if raw is not None: patch["nozzle_mapping"] = json.dumps(raw) - ams_mapping_json = _extract_slicer_ams_mapping_json(data, f"[VP {self.name}] Late MQTT") + # Same target_printer_id gate as the immediate path in + # _add_to_print_queue — a model-based VP has no MQTT bridge to a real + # printer, so there's no live AMS layout for the slicer to have + # resolved tray IDs against. + ams_mapping_json = ( + _extract_slicer_ams_mapping_json(data, f"[VP {self.name}] Late MQTT") + if self.target_printer_id is not None + else None + ) if ams_mapping_json is not None: patch["ams_mapping"] = ams_mapping_json @@ -566,7 +574,10 @@ class VirtualPrinterInstance: archive_result = await db.execute(select(PrintArchive).where(PrintArchive.id.in_(archive_ids))) for archive in archive_result.scalars().all(): extra = dict(archive.extra_data or {}) - extra["slicer_ams_mapping"] = json.loads(ams_mapping_json) + extra["slicer_ams_mapping"] = { + "mapping": json.loads(ams_mapping_json), + "printer_id": self.target_printer_id, + } archive.extra_data = extra await db.commit() @@ -917,8 +928,17 @@ class VirtualPrinterInstance: # present it makes `_ensure_ams_mapping` skip its own # type/color re-derivation entirely and dispatch use exactly # the tray the slicer/user picked. + # + # Only trust it when this VP targets one fixed printer. A + # model-based ("Any ") VP has no MQTT bridge to a real + # printer, so the slicer has no live AMS layout to resolve + # tray IDs against — whatever it sends here is meaningless + # (or, worse, coincidentally valid for the wrong printer once + # the scheduler later picks one). Leaving it unset lets the + # scheduler's normal type/color re-derivation run against + # whichever printer actually gets the job. ams_mapping_json: str | None = None - if slicer_opts is not None: + if slicer_opts is not None and self.target_printer_id is not None: ams_mapping_json = _extract_slicer_ams_mapping_json(slicer_opts, f"[VP {self.name}]") service = ArchiveService(db) @@ -929,19 +949,22 @@ class VirtualPrinterInstance: "status": "archived", "source": "virtual_printer", "source_ip": source_ip, - # Slicer's own live AMS-slot pick -- promoted to - # `extra_data.slicer_ams_mapping` by archive_print() so - # a later reprint can reuse it. Opt-in per VP - # (`save_ams_mapping`) — only the archive persistence is - # gated; the queue item's own `ams_mapping` (used for - # *this* dispatch, below) is set unconditionally - # whenever the slicer provides it, since that's a - # correctness fix, not a feature toggle. - "ams_mapping": ( - json.loads(ams_mapping_json) if ams_mapping_json and self.save_ams_mapping else None - ), }, prefer_filename_for_name=prefer_filename, + # Slicer's own live AMS-slot pick -- promoted to + # `extra_data.slicer_ams_mapping` by archive_print() so a + # later reprint can reuse it. Opt-in per VP + # (`save_ams_mapping`) — only the archive persistence is + # gated; the queue item's own `ams_mapping` (used for + # *this* dispatch, below) is set unconditionally whenever + # the slicer provides it, since that's a correctness fix, + # not a feature toggle. Tagged with the printer it was + # resolved against so a later reprint on a *different* + # printer knows not to reuse it (#2700 review). + slicer_ams_mapping=( + json.loads(ams_mapping_json) if ams_mapping_json and self.save_ams_mapping else None + ), + slicer_ams_mapping_printer_id=self.target_printer_id, ) if archive: logger.info("[VP %s] Archived: %s - %s", self.name, archive.id, archive.print_name) diff --git a/backend/tests/integration/test_print_queue_api.py b/backend/tests/integration/test_print_queue_api.py index 44af572f3..728a1970c 100644 --- a/backend/tests/integration/test_print_queue_api.py +++ b/backend/tests/integration/test_print_queue_api.py @@ -264,13 +264,16 @@ class TestPrintQueueAPI: self, async_client: AsyncClient, printer_factory, archive_factory, db_session ): """When the caller sends no explicit ams_mapping, but the archive - carries the slicer's own saved pick (extra_data.slicer_ams_mapping, - written by a VP with "Save AMS mapping" on), the queue item should - inherit it — the same exact-physical-spool reuse the "Mapping" - button gives you, but automatic when nothing was hand-edited. + carries the slicer's own saved pick for this exact printer + (extra_data.slicer_ams_mapping, written by a VP with "Save AMS + mapping" on), the queue item should inherit it — the same + exact-physical-spool reuse the "Mapping" button gives you, but + automatic when nothing was hand-edited. """ printer = await printer_factory() - archive = await archive_factory(extra_data={"slicer_ams_mapping": [5, -1, 2, -1]}) + archive = await archive_factory( + extra_data={"slicer_ams_mapping": {"mapping": [5, -1, 2, -1], "printer_id": printer.id}} + ) data = { "printer_id": printer.id, @@ -281,6 +284,54 @@ class TestPrintQueueAPI: result = response.json() assert result["ams_mapping"] == [5, -1, 2, -1] + @pytest.mark.asyncio + @pytest.mark.integration + async def test_add_to_queue_ignores_archive_slicer_ams_mapping_for_different_printer( + self, async_client: AsyncClient, printer_factory, archive_factory, db_session + ): + """A saved mapping's tray IDs only mean something relative to the + printer they were resolved against. Reprinting the same archive on a + *different* printer must not inherit it — tray 5 on printer A can + hold a completely different spool than tray 5 on printer B. + """ + origin_printer = await printer_factory() + other_printer = await printer_factory() + archive = await archive_factory( + extra_data={"slicer_ams_mapping": {"mapping": [5, -1, 2, -1], "printer_id": origin_printer.id}} + ) + + data = { + "printer_id": other_printer.id, + "archive_id": archive.id, + } + response = await async_client.post("/api/v1/queue/", json=data) + assert response.status_code == 200 + result = response.json() + assert result["ams_mapping"] is None + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_add_to_queue_ignores_archive_slicer_ams_mapping_for_model_based_dispatch( + self, async_client: AsyncClient, printer_factory, archive_factory, db_session + ): + """A model-based item (no fixed printer_id) can't know in advance + which printer the scheduler will pick, so a saved mapping resolved + against one specific printer must never be inherited here either. + """ + origin_printer = await printer_factory() + archive = await archive_factory( + extra_data={"slicer_ams_mapping": {"mapping": [5, -1, 2, -1], "printer_id": origin_printer.id}} + ) + + data = { + "target_model": "X1C", + "archive_id": archive.id, + } + response = await async_client.post("/api/v1/queue/", json=data) + assert response.status_code == 200 + result = response.json() + assert result["ams_mapping"] is None + @pytest.mark.asyncio @pytest.mark.integration async def test_add_to_queue_explicit_ams_mapping_wins_over_archive_fallback( @@ -291,7 +342,9 @@ class TestPrintQueueAPI: pick — the fallback only fires when the caller sent nothing at all. """ printer = await printer_factory() - archive = await archive_factory(extra_data={"slicer_ams_mapping": [5, -1, 2, -1]}) + archive = await archive_factory( + extra_data={"slicer_ams_mapping": {"mapping": [5, -1, 2, -1], "printer_id": printer.id}} + ) data = { "printer_id": printer.id, diff --git a/backend/tests/unit/services/test_virtual_printer.py b/backend/tests/unit/services/test_virtual_printer.py index cf02e621c..36e521e86 100644 --- a/backend/tests/unit/services/test_virtual_printer.py +++ b/backend/tests/unit/services/test_virtual_printer.py @@ -1880,6 +1880,7 @@ class TestVirtualPrinterInstance: base_dir=tmp_path, session_factory=mock_session_factory, save_ams_mapping=False, + target_printer_id=7, ) file_path = tmp_path / "test.3mf" @@ -1913,9 +1914,8 @@ class TestVirtualPrinterInstance: assert item.ams_mapping is not None assert _json.loads(item.ams_mapping) == [4, -1, 12, -1] - # Toggle is off: the archive's print_data must NOT carry the mapping. - print_data = mock_archive_print.await_args.kwargs["print_data"] - assert print_data["ams_mapping"] is None + # Toggle is off: archive_print must not be told to persist a mapping. + assert mock_archive_print.await_args.kwargs["slicer_ams_mapping"] is None @pytest.mark.asyncio async def test_add_to_print_queue_persists_ams_mapping_to_archive_when_toggle_on(self, tmp_path): @@ -1948,6 +1948,7 @@ class TestVirtualPrinterInstance: base_dir=tmp_path, session_factory=mock_session_factory, save_ams_mapping=True, + target_printer_id=7, ) file_path = tmp_path / "test.3mf" @@ -1979,8 +1980,75 @@ class TestVirtualPrinterInstance: assert len(added_items) == 1 assert _json.loads(added_items[0].ams_mapping) == [4, -1, 12, -1] - print_data = mock_archive_print.await_args.kwargs["print_data"] - assert print_data["ams_mapping"] == [4, -1, 12, -1] + assert mock_archive_print.await_args.kwargs["slicer_ams_mapping"] == [4, -1, 12, -1] + assert mock_archive_print.await_args.kwargs["slicer_ams_mapping_printer_id"] == 7 + + @pytest.mark.asyncio + async def test_add_to_print_queue_ignores_ams_mapping_for_model_based_vp(self, tmp_path): + """A model-based VP (`target_printer_id=None`, dispatched later by the + scheduler to whichever printer matches) has no MQTT bridge to a real + printer, so the slicer has no live AMS layout to resolve tray IDs + against. Whatever it sends must be ignored on both the queue item and + the archive — trusting it would dispatch the eventually-chosen + printer onto a tray resolved against nothing (#2700 review). + """ + import json as _json + + from backend.app.services.virtual_printer.manager import VirtualPrinterInstance + + added_items = [] + mock_db = AsyncMock() + mock_db.add = MagicMock(side_effect=added_items.append) + mock_db.commit = AsyncMock() + mock_session_factory = MagicMock() + mock_session_ctx = AsyncMock() + mock_session_ctx.__aenter__ = AsyncMock(return_value=mock_db) + mock_session_ctx.__aexit__ = AsyncMock(return_value=False) + mock_session_factory.return_value = mock_session_ctx + + inst = VirtualPrinterInstance( + vp_id=451, + name="AMSMappingModelBased", + mode="queue", + model="X1C", + access_code="12345678", + serial_suffix="391800451", + base_dir=tmp_path, + session_factory=mock_session_factory, + save_ams_mapping=True, + target_printer_id=None, + ) + + file_path = tmp_path / "test.3mf" + file_path.write_bytes(b"fake3mf") + + await inst.on_print_command( + file_path.name, + {"command": "project_file", "ams_mapping": [4, -1, 12, -1]}, + ) + + mock_archive = MagicMock() + mock_archive.id = 1 + mock_archive.print_name = "test" + + with ( + patch( + "backend.app.api.routes.settings.get_setting", + new_callable=AsyncMock, + return_value=None, + ), + patch( + "backend.app.services.archive.ArchiveService.archive_print", + new_callable=AsyncMock, + return_value=mock_archive, + ) as mock_archive_print, + ): + await inst._add_to_print_queue(file_path, "192.168.1.100") + + assert len(added_items) == 1 + assert added_items[0].ams_mapping is None + assert mock_archive_print.await_args.kwargs["slicer_ams_mapping"] is None + assert mock_archive_print.await_args.kwargs["slicer_ams_mapping_printer_id"] is None @pytest.mark.asyncio async def test_add_to_print_queue_ignores_unresolved_ams_mapping_sentinel(self, tmp_path): @@ -2010,6 +2078,7 @@ class TestVirtualPrinterInstance: base_dir=tmp_path, session_factory=mock_session_factory, save_ams_mapping=True, + target_printer_id=7, ) file_path = tmp_path / "test.3mf" @@ -2040,8 +2109,7 @@ class TestVirtualPrinterInstance: assert len(added_items) == 1 assert added_items[0].ams_mapping is None - print_data = mock_archive_print.await_args.kwargs["print_data"] - assert print_data["ams_mapping"] is None + assert mock_archive_print.await_args.kwargs["slicer_ams_mapping"] is None @pytest.mark.asyncio async def test_add_to_print_queue_nozzle_pick_replicated_across_plates(self, tmp_path, monkeypatch): @@ -2228,6 +2296,205 @@ class TestVirtualPrinterInstance: # Recent-queue tracking dict is cleared after the patch. assert file_path.name not in inst._recent_queue_items + @pytest.mark.asyncio + async def test_on_print_command_late_mqtt_retroactively_stamps_archive_ams_mapping(self, tmp_path): + """Same #1780-round-3 race as the test above, but for `ams_mapping` + specifically: the archive was already created (with no + `slicer_ams_mapping`, since the slicer's pick hadn't arrived yet) + before this late MQTT lands. With the toggle on and a fixed target + printer, the archive must be retroactively patched too — otherwise a + reprint from this archive would never see the badge or the Mapping + button, even though the queue item itself dispatches correctly. + """ + import json as _json + + from backend.app.services.virtual_printer.manager import VirtualPrinterInstance + + added_items: list = [] + mock_db = AsyncMock() + mock_db.add = MagicMock( + side_effect=lambda item: (added_items.append(item), setattr(item, "id", 100 + len(added_items)))[0] + ) + mock_db.flush = AsyncMock() + mock_db.commit = AsyncMock() + + mock_archive_row = MagicMock() + mock_archive_row.id = 55 + mock_archive_row.extra_data = None + + position_max_result = MagicMock() + position_max_result.scalar = MagicMock(return_value=None) + select_pending_result = MagicMock() + # (queue_item_id, archive_id) — matches the id set by db.add above. + select_pending_result.all = MagicMock(return_value=[(101, 55)]) + update_result = MagicMock() + select_archives_result = MagicMock() + select_archives_result.scalars = MagicMock(return_value=MagicMock(all=MagicMock(return_value=[mock_archive_row]))) + mock_db.execute = AsyncMock( + side_effect=[position_max_result, select_pending_result, update_result, select_archives_result] + ) + + mock_session_factory = MagicMock() + mock_session_ctx = AsyncMock() + mock_session_ctx.__aenter__ = AsyncMock(return_value=mock_db) + mock_session_ctx.__aexit__ = AsyncMock(return_value=False) + mock_session_factory.return_value = mock_session_ctx + + inst = VirtualPrinterInstance( + vp_id=98, + name="LateMQTTArchivePatch", + mode="queue", + model="X1C", + access_code="12345678", + serial_suffix="391800098", + base_dir=tmp_path, + session_factory=mock_session_factory, + save_ams_mapping=True, + target_printer_id=7, + ) + inst._mqtt = MagicMock() + + file_path = tmp_path / "test.3mf" + file_path.write_bytes(b"fake3mf") + + mock_archive = MagicMock() + mock_archive.id = 55 + mock_archive.print_name = "test" + + with ( + patch( + "backend.app.api.routes.settings.get_setting", + new_callable=AsyncMock, + return_value=None, + ), + patch( + "backend.app.services.archive.ArchiveService.archive_print", + new_callable=AsyncMock, + return_value=mock_archive, + ), + patch( + "backend.app.services.virtual_printer.manager._SLICER_OPTIONS_WAIT_TIMEOUT", + 0.05, + ), + ): + await inst._add_to_print_queue(file_path, "192.168.1.100") + + assert len(added_items) == 1 + assert added_items[0].ams_mapping is None # MQTT was never received in time + + # MQTT project_file arrives late, carrying the slicer's AMS pick. + await inst.on_print_command( + file_path.name, + { + "command": "project_file", + "file": file_path.name, + "ams_mapping": [4, -1, 12, -1], + }, + ) + + # 3rd execute() is the queue-item UPDATE; 4th is the archive SELECT. + update_call = mock_db.execute.await_args_list[2] + compiled = update_call.args[0].compile(compile_kwargs={"literal_binds": False}) + assert _json.loads(dict(compiled.params)["ams_mapping"]) == [4, -1, 12, -1] + + assert mock_archive_row.extra_data == { + "slicer_ams_mapping": {"mapping": [4, -1, 12, -1], "printer_id": 7}, + } + + @pytest.mark.asyncio + async def test_on_print_command_late_mqtt_skips_archive_patch_for_model_based_vp(self, tmp_path): + """The archive patch above must not fire for a model-based VP + (`target_printer_id=None`) either — same rationale as the immediate + path: no fixed printer means no live AMS layout to have resolved the + late mapping against. + """ + import json as _json + + from backend.app.services.virtual_printer.manager import VirtualPrinterInstance + + added_items: list = [] + mock_db = AsyncMock() + mock_db.add = MagicMock( + side_effect=lambda item: (added_items.append(item), setattr(item, "id", 200 + len(added_items)))[0] + ) + mock_db.flush = AsyncMock() + mock_db.commit = AsyncMock() + + position_max_result = MagicMock() + position_max_result.scalar = MagicMock(return_value=None) + select_pending_result = MagicMock() + select_pending_result.all = MagicMock(return_value=[(201, 65)]) + update_result = MagicMock() + # No 4th execute() expected — the archive patch must be skipped + # entirely, so only 3 calls should ever happen. + mock_db.execute = AsyncMock(side_effect=[position_max_result, select_pending_result, update_result]) + + mock_session_factory = MagicMock() + mock_session_ctx = AsyncMock() + mock_session_ctx.__aenter__ = AsyncMock(return_value=mock_db) + mock_session_ctx.__aexit__ = AsyncMock(return_value=False) + mock_session_factory.return_value = mock_session_ctx + + inst = VirtualPrinterInstance( + vp_id=97, + name="LateMQTTModelBased", + mode="queue", + model="X1C", + access_code="12345678", + serial_suffix="391800097", + base_dir=tmp_path, + session_factory=mock_session_factory, + save_ams_mapping=True, + target_printer_id=None, + ) + inst._mqtt = MagicMock() + + file_path = tmp_path / "test.3mf" + file_path.write_bytes(b"fake3mf") + + mock_archive = MagicMock() + mock_archive.id = 65 + mock_archive.print_name = "test" + + with ( + patch( + "backend.app.api.routes.settings.get_setting", + new_callable=AsyncMock, + return_value=None, + ), + patch( + "backend.app.services.archive.ArchiveService.archive_print", + new_callable=AsyncMock, + return_value=mock_archive, + ), + patch( + "backend.app.services.virtual_printer.manager._SLICER_OPTIONS_WAIT_TIMEOUT", + 0.05, + ), + ): + await inst._add_to_print_queue(file_path, "192.168.1.100") + + await inst.on_print_command( + file_path.name, + { + "command": "project_file", + "file": file_path.name, + "ams_mapping": [4, -1, 12, -1], + # An unrelated field so `patch` isn't empty and the UPDATE + # actually runs — isolates the assertion to "ams_mapping was + # excluded" rather than "nothing happened at all". + "timelapse": True, + }, + ) + + # 3rd execute() is the queue-item UPDATE — ams_mapping must be absent + # from it. No 4th execute() (the archive SELECT) should follow. + assert mock_db.execute.await_count == 3 + update_call = mock_db.execute.await_args_list[2] + compiled = update_call.args[0].compile(compile_kwargs={"literal_binds": False}) + assert "ams_mapping" not in dict(compiled.params) + assert dict(compiled.params)["timelapse"] is True + @pytest.mark.asyncio async def test_add_to_print_queue_catches_mqtt_stashed_post_wait_timeout(self, tmp_path): """The actual race-window scenario: wait_for times out, then MQTT diff --git a/frontend/src/components/PrintModal/FilamentMapping.tsx b/frontend/src/components/PrintModal/FilamentMapping.tsx index acd8854c1..c8df976cb 100644 --- a/frontend/src/components/PrintModal/FilamentMapping.tsx +++ b/frontend/src/components/PrintModal/FilamentMapping.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useState } from 'react'; +import { useEffect, useMemo, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import { Circle, Check, AlertTriangle, RefreshCw, ChevronDown, ChevronUp, Palette } from 'lucide-react'; @@ -38,34 +38,46 @@ export function FilamentMapping({ // normal auto-match, without touching any *other* manual picks the user // made by hand. const [usingArchiveMapping, setUsingArchiveMapping] = useState(false); + // Which slot IDs the ON branch below actually wrote into manualMappings — + // so OFF can undo exactly those and leave any *other* manual pick the user + // made by hand (before or after pressing the button) untouched. + const appliedSlotIdsRef = useRef([]); // Reset the toggle whenever the saved mapping it would apply changes — a - // different plate selection or a different archive entirely. Without this - // the button can read ON (green) from a previous archive/plate even though - // it was never pressed against the mapping currently in scope. + // different printer, plate selection, or archive entirely. Without this + // the button can read ON (green) from a previous printer/archive/plate + // even though it was never pressed against the mapping currently in scope. useEffect(() => { setUsingArchiveMapping(false); - }, [archiveAmsMapping, plateLabel]); + appliedSlotIdsRef.current = []; + }, [archiveAmsMapping, plateLabel, printerId]); const toggleArchiveMapping = () => { if (!archiveAmsMapping || !filamentReqs?.filaments) return; if (usingArchiveMapping) { const next = { ...manualMappings }; - for (const req of filamentReqs.filaments) { - if (req.slot_id > 0) delete next[req.slot_id]; + for (const slotId of appliedSlotIdsRef.current) { + delete next[slotId]; } onManualMappingChange(next); + appliedSlotIdsRef.current = []; setUsingArchiveMapping(false); return; } const next = { ...manualMappings }; + const appliedSlotIds: number[] = []; for (const req of filamentReqs.filaments) { const idx = req.slot_id - 1; + // A negative value (e.g. the external spool sentinel) means the + // slicer didn't resolve this filament to an AMS tray — leave that + // slot's existing auto-match/manual pick alone rather than clearing it. if (req.slot_id > 0 && idx >= 0 && idx < archiveAmsMapping.length && archiveAmsMapping[idx] >= 0) { next[req.slot_id] = archiveAmsMapping[idx]; + appliedSlotIds.push(req.slot_id); } } onManualMappingChange(next); + appliedSlotIdsRef.current = appliedSlotIds; setUsingArchiveMapping(true); }; diff --git a/frontend/src/components/PrintModal/index.tsx b/frontend/src/components/PrintModal/index.tsx index 1067e1954..d278baa2a 100644 --- a/frontend/src/components/PrintModal/index.tsx +++ b/frontend/src/components/PrintModal/index.tsx @@ -327,10 +327,21 @@ export function PrintModal({ // The archive's own saved AMS-slot pick from the slicer (see the "Save AMS // mapping" virtual-printer setting) — undefined for library files or - // archives that predate the feature / had it off at print time. - const archiveSlicerAmsMapping = !isLibraryFile - ? (archiveDetails?.extra_data?.slicer_ams_mapping as number[] | undefined) + // archives that predate the feature / had it off at print time. Stored as + // `{mapping, printer_id}`: the tray IDs are only meaningful against the + // specific printer's AMS layout they were resolved from, so only surface + // them when reprinting on that same printer — a different printer's slot 3 + // can hold a completely different spool. + const archiveSlicerAmsMappingRaw = !isLibraryFile + ? (archiveDetails?.extra_data?.slicer_ams_mapping as + | { mapping?: number[]; printer_id?: number } + | undefined) : undefined; + const archiveSlicerAmsMapping = + archiveSlicerAmsMappingRaw?.printer_id === effectivePrinterId && + Array.isArray(archiveSlicerAmsMappingRaw.mapping) + ? archiveSlicerAmsMappingRaw.mapping + : undefined; // Fetch plates for archives const { data: archivePlatesData, isError: archivePlatesError } = useQuery({ @@ -1441,6 +1452,7 @@ export function PrintModal({ onForceColorMatchChange={(slotId, value) => setForceColorMatch((prev) => ({ ...prev, [slotId]: value })) } + archiveAmsMapping={archiveSlicerAmsMapping} /> ); })} diff --git a/frontend/src/pages/ArchivesPage.tsx b/frontend/src/pages/ArchivesPage.tsx index 4aa2a4e6b..5f1fec7ee 100644 --- a/frontend/src/pages/ArchivesPage.tsx +++ b/frontend/src/pages/ArchivesPage.tsx @@ -1113,7 +1113,13 @@ function ArchiveCard({ {/* Slicer's own saved AMS-slot pick (see "Save AMS mapping" VP setting) — a reprint reuses this exact physical spool instead of re-deriving one from the file's type/color. */} - {Array.isArray((archive.extra_data as Record | null)?.slicer_ams_mapping) && ( + {Array.isArray( + ( + (archive.extra_data as Record | null)?.slicer_ams_mapping as + | { mapping?: unknown } + | undefined + )?.mapping, + ) && (
{t('archives.card.slicerAmsMapping')}