fix(notifications): reprint-from-archive sent original print's finish photo (#1707)

Reprints reuse the source archive row via the expected-print promotion
  branch, but that branch never reset archive.timelapse_path. The stale
  path made _scan_for_timelapse_with_retries early-return (so the new
  run's MP4 was never downloaded), and _capture_finish_photo_from_timelapse
  then extracted the *original* run's last frame and shipped it to Telegram
  as the new run's finish photo. Surface was specific to the
  timelapse-prefer path (data.timelapse_was_active=true and no external
  camera) — fallback live-camera paths were unaffected, which is why this
  took a P2S reporter to surface.

  Clear archive.timelapse_path at promotion and unlink the stale on-disk
  MP4 (best-effort; missing files are logged and skipped). The orphan
  unlink also kills a long-standing file-leak: every reprint used to leave
  its predecessor's timelapse on disk forever. archive.photos is
  deliberately left alone — accumulating one finish photo per run is
  correct.
This commit is contained in:
maziggy
2026-06-11 09:23:03 +02:00
parent 6316ca929e
commit bf781e0566
3 changed files with 355 additions and 0 deletions
+2
View File
@@ -24,6 +24,8 @@ All notable changes to Bambuddy will be documented in this file.
- **Print Log page: per-row delete (#1687 part 1, reported by @IndividualGhost1905)** — Reporter noted that the existing "Also remove this print from Quick Stats" toggle on archive delete is one-shot: if you tick "keep stats" at delete time, there was no later way to drop the row from /stats; and rows that aren't tied to an archive (errors, aborts, manual entries) had no delete affordance at all. **Fix:** every row in the Archives → Print Log table now has a trash icon next to the filament cell, gated on `archives:delete_own` (own rows) or `archives:delete_all` (any row), matching the archive-delete permission shape. Click → confirm modal → row is gone, and because /archives/stats aggregates over `PrintLogEntry` the filament / time / cost contribution drops out of Quick Stats in the same response cycle. The matching archive (if any) is untouched — the log row is a sibling, not a child. **Backend:** new `DELETE /print-log/{entry_id}` mirrors `delete_archive`'s ownership flow via `require_ownership_permission(ARCHIVES_DELETE_ALL, ARCHIVES_DELETE_OWN)`; owners can drop their own rows, admins can drop any row, missing IDs return 404 rather than 200-silently. **Frontend:** new `deletePrintLogEntry` API helper, per-row mutation that invalidates both `print-log` and `archives-stats` query keys so the totals re-render without a manual refresh. **i18n:** 4 new keys (`deleteEntryTitle`, `deleteEntryConfirm`, `entryDeleted`, `entryDeleteFailed`) translated across all 11 locales (de / en / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW). **Tests:** 3 backend integration cases — delete drops the row from /stats while keeping the linked archive listed, missing ID returns 404, delete-one does not touch siblings (regression guard against an accidental `delete(PrintLogEntry)` without a `where`). Frontend ArchivesPage / PrintLogModal vitests stay green (31 / 31). i18n parity green (5099 leaves × 11 locales). Issue #1687 also asks for per-row tagging (already covered by `EditArchiveModal`'s tags field) and per-row filament-usage-history edits (deferred — see the issue thread for the reasoning).
### Fixed
- **Telegram (and other image-bearing) finish notification on a reprint-from-archive showed the original print's finish photo instead of the new run's (#1707, reported by @kycrna)** — P2S user reprinted an archived job and observed the Telegram notification arriving with the photo of the *original* print (white box) attached to the completion message for the *new* run (black box). **Root cause:** reprints reuse the source archive row — `register_expected_print` stores the source `archive_id` in `_expected_prints`, and the on-print-start expected-archive promotion branch at `main.py:2207-2245` updates the row's status / started_at / printer_id / subtask_id but never reset `archive.timelapse_path`. Two failure modes cascaded from the stale path: (a) `_scan_for_timelapse_with_retries` early-returns at `main.py:3062` with `if archive.timelapse_path: return` — the reprint's new timelapse MP4 sitting on the printer's SD card was never downloaded, the archive's `timelapse_path` kept pointing at the original run's local file; (b) `_capture_finish_photo_from_timelapse` polls `archive.timelapse_path` and immediately found the *original* video, extracted ITS last frame as `finish_<fresh-timestamp>_<uuid>.jpg`, and handed those bytes to `_background_notifications` as `image_data` — which then went out to Telegram via the `sendPhoto` path. The filename was new (so log lines and the archive's `photos` list looked correct), but the pixels were the original run's finish frame. Surface was specific to the timelapse-prefer path: with `data.timelapse_was_active` true and no external camera, `prefer_timelapse_source` was True, which is the exact configuration on P2S with timelapse-on for both runs. External-camera, buffered-frame, and fresh-RTSP fallback paths grab the *current* camera frame, so users on those paths saw correct photos and the bug stayed hidden. **Fix:** at expected-archive promotion, capture and clear `archive.timelapse_path` to None before the commit, and `os.unlink` the stale on-disk video so reprints don't accumulate orphaned MP4s in the archive directory. Photos list is left alone — accumulating one finish photo per run across the archive's lifetime is the right behaviour. The unlink is wrapped in `OSError`-catching best-effort logging so a missing file (manual delete, archive purge, container rebuild with bind-mount drift) doesn't break promotion. The clear-and-unlink runs unconditionally when `timelapse_path` is set, so even if a user has been reprinting under the buggy build for months, the next reprint self-heals. **Tests:** 3 new cases in `test_reprint_clears_stale_timelapse.py` exercise the full `on_print_start` callback through the expected-archive branch — happy path (path cleared + file unlinked), no-prior-timelapse (no-op, promotion still succeeds), missing-stale-file (best-effort unlink doesn't raise). Full `test_print_start_expected_promotion.py` + `test_print_start_assigns_printer_id_to_vp_archive.py` suite (28/28) stays green; ruff clean.
- **Connection diagnostic no longer flags `external_storage: fail` on A1 / A1 Mini, which physically have no MicroSD slot (#1703, reported by @MartinNYHC)** — Bug report from an A1 user complained that BambuStudio and OrcaSlicer don't have an "external storage" tick box (correct — there's nothing to toggle, the A1 series ships without a SD card slot at all) while the Bambuddy support bundle simultaneously reported `external_storage: fail` in the printer's connection diagnostic. The two together left the user thinking Bambuddy was wrong about a setting their hardware doesn't have. **Root cause:** the `external_storage` check at `services/printer_diagnostic.py:179-189` reads `state.store_to_sdcard`, which is parsed from MQTT `home_flag` bit 11. On A1 and A1 Mini that bit is never set (no hardware slot, no firmware-side toggle, no slicer-side equivalent), so the value pushes as `False` and the check fell through to `fail` instead of `skip`. **Fix:** new `NO_EXTERNAL_STORAGE_MODELS` frozenset in `utils/printer_models.py` enumerating A1, A1 Mini, and their internal codes (N1, N2S, A04, A11, A12), plus a `has_external_storage(model)` helper that returns False for those and True for everything else (unknown models default to True so the check stays active for any future Bambu model that ships *with* a slot — new no-slot models must be added to the set explicitly). The diagnostic now short-circuits to `skip` before reading `store_to_sdcard` when `printer.model` is in the set. **What this does NOT change:** X1 / X1E / P1S / P1P / P2S / H2D / H2D Pro / H2C / H2S / X2D continue to evaluate `store_to_sdcard` exactly as before — the home-flag-bit-off → `fail` path is still the right signal for them. **The companion FTP-upload-timeout symptom in the same bug report (ftp code 28 from BambuStudio when sending to the proxy VP) is a separate Docker-bridge-mode networking constraint, not addressed by this change.** **Tests:** 8 new cases — `TestHasExternalStorage` (5 cases) pins the model list, internal-code aliasing, case/whitespace normalisation, unknown-defaults-true, and null/empty-defaults-true; `TestExternalStorageCheck` gains `test_skips_on_a1_no_external_storage_slot`, `test_skips_on_a1_mini_no_external_storage_slot`, and `test_still_fails_on_x1c_when_toggle_off` (regression guard that the model-aware skip doesn't accidentally silence the genuine signal on slotted models). Full `test_printer_models.py` + `test_printer_diagnostic.py` + archives integration suite green (172/172); ruff clean.
- **AMS slot card surfaced the previous spool's preset name after RFID auto-assigned a new spool (reported with H2D-1 / AMS-B3 / PLA-CF showing as "Bambu PLA Silk+")** — Reporter inserted a fresh Bambu PLA-CF spool into AMS-B3, RFID identified it correctly, but the slot card kept showing "Bambu PLA Silk+" (the name from a PLA Silk+ spool that had occupied the slot back in March). Confirmed in the live data: `slot_preset_mappings` row for `(printer_id=1, ams_id=1, tray_id=2)` was `preset_id=GFSA06_09, preset_name='Bambu PLA Silk+', updated_at=2026-03-15` — three months stale. **Root cause:** `slot_preset_mappings.preset_name` is first in the PrintersPage display chain (`PrintersPage.tsx:3624`) and overrides the spool's own `slicer_filament_name` plus the cloud catalog `cloudInfo.name`. The internal-mode manual-assign path (`inventory.apply_spool_to_slot_via_mqtt`) kept this row in sync, but the internal-mode RFID auto-assign path (`spool_tag_matcher.auto_assign_spool`) skipped it entirely. The Spoolman-mode sync path (`main.auto_sync_spoolman_ams_trays`) also skipped it — same bug shape, latent for Spoolman users who'd never manually configured a slot preset, active for those who had. **Fix — three writers in lockstep via one shared helper.** New `backend/app/services/slot_preset_writer.py` exposes a primitive `upsert_slot_preset` plus two convenience wrappers: `upsert_slot_preset_for_spool` for internal `Spool` ORM objects (local-preset numeric ids → `local_{n}`, cloud ids run through `filament_id_to_setting_id`) and `upsert_slot_preset_for_spoolman_spool` for Spoolman dicts (filament.name → preset_name, tray_info_idx → preset_id). All three call sites — the manual-assign block in `inventory.py:396-438`, the RFID auto-assign tail in `spool_tag_matcher.py:auto_assign_spool`, and the per-tray-sync branch in `main.py:auto_sync_spoolman_ams_trays` — now go through the helper. **Self-heal:** existing stale rows from past spool swaps get rewritten the next time a fresh spool is detected on the same slot. No migration script needed. **What this also covers per `feedback_inventory_modes_parity`:** the bug shape exists in both internal and Spoolman modes, so the patch ships fixes for both inventory paths in the same drop — a Spoolman user with a manually-configured slot preset would have seen the same stale-name behavior after every RFID swap until the row was overwritten through Configure Slot. **Tests:** new `test_slot_preset_writer.py` (6 cases) pins the helper contracts — no-op on empty preset_id, upsert idempotency, Spoolman filament.name → preset_name, fallback to material → tray_sub_brands → tray_type, stale-row overwrite from the Spoolman path, skip when tray_info_idx is unknown. New `test_spool_tag_matcher.py` cases (3) pin the internal RFID-auto-assign path — stale-row overwrite (the exact reporter shape: PLA Silk+ → PLA-CF), fresh insert when no row exists, `local_{n}` formatting for numeric local-preset ids. Total touched-area suite 69/69 green; broader related suite (inventory + spoolman + spool_tag + auto_sync) 767/767 green; ruff clean.
+27
View File
@@ -2216,6 +2216,33 @@ async def on_print_start(printer_id: int, data: dict):
# Update archive status to printing
archive.status = "printing"
archive.started_at = datetime.now(timezone.utc)
# Reprint of an archive reuses the source row. Without resetting
# ``timelapse_path`` _scan_for_timelapse_with_retries early-returns
# ("already has timelapse") and _capture_finish_photo_from_timelapse
# extracts the *original* print's last frame, which then ships in
# the completion notification (#1707). Clear the path so the
# scanner runs fresh; also unlink the old video file so reprints
# don't accumulate orphans in the archive directory. Photos list
# is left alone — accumulating one finish photo per run is fine.
stale_timelapse_relpath = archive.timelapse_path
if stale_timelapse_relpath:
archive.timelapse_path = None
try:
stale_path = app_settings.base_dir / stale_timelapse_relpath
if stale_path.is_file():
stale_path.unlink()
logger.info(
"Deleted stale timelapse %s on reprint of archive %s",
stale_timelapse_relpath,
expected_archive_id,
)
except OSError as e:
logger.warning(
"Failed to delete stale timelapse %s on reprint: %s",
stale_timelapse_relpath,
e,
)
# Persist a restart-stable id so a later restart resumes this
# archive by subtask_id instead of name-matching + duplicating
# it (#1485). The printer often hasn't echoed subtask_id back
@@ -0,0 +1,326 @@
"""Regression for #1707: Telegram (and any image-bearing) notification on a
reprint from archive showed the *original* print's finish photo because the
expected-archive branch never reset ``archive.timelapse_path``.
The source archive row is reused for reprints. With ``timelapse_path`` still
pointing at the original run's downloaded MP4:
- ``_scan_for_timelapse_with_retries`` early-returns ("already has timelapse")
and never downloads the reprint's video.
- ``_capture_finish_photo_from_timelapse`` reads the stale path, extracts the
*original* last frame, and ships it as the reprint's finish photo.
The fix clears ``archive.timelapse_path`` (and unlinks the stale file) at
expected-archive promotion so the scan + photo path run fresh.
"""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from backend.app.core.config import settings as app_settings
from backend.app.main import (
_active_prints,
_expected_print_creators,
_expected_print_registered_at,
_expected_prints,
_print_ams_mappings,
_timelapse_baselines,
register_expected_print,
)
@pytest.fixture(autouse=True)
def _clear_dicts():
_expected_prints.clear()
_expected_print_registered_at.clear()
_expected_print_creators.clear()
_print_ams_mappings.clear()
_active_prints.clear()
_timelapse_baselines.clear()
yield
_expected_prints.clear()
_expected_print_registered_at.clear()
_expected_print_creators.clear()
_print_ams_mappings.clear()
_active_prints.clear()
_timelapse_baselines.clear()
def _patches():
"""Common patches for driving on_print_start without side effects."""
return (
patch("backend.app.main.async_session"),
patch("backend.app.main.notification_service"),
patch("backend.app.main.smart_plug_manager"),
patch("backend.app.main.ws_manager"),
patch("backend.app.main.printer_manager"),
patch("backend.app.main.mqtt_relay"),
patch("backend.app.main._record_energy_start", new_callable=AsyncMock),
patch("backend.app.main._load_objects_from_archive"),
patch("backend.app.main._store_spoolman_print_data", new_callable=AsyncMock),
patch("backend.app.main._send_print_start_notification", new_callable=AsyncMock),
patch(
"backend.app.main._list_timelapse_videos",
new=AsyncMock(return_value=([], "/timelapse")),
),
)
def _build_mocks(mock_printer, mock_archive):
def execute_router(stmt, *args, **kwargs):
sql = str(stmt).lower()
if "from printers" in sql or "from printer " in sql:
return MagicMock(
scalar_one_or_none=MagicMock(return_value=mock_printer),
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[mock_printer]))),
)
if "from print_archives" in sql or "from print_archive" in sql:
return MagicMock(
scalar_one_or_none=MagicMock(return_value=mock_archive),
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[mock_archive]))),
)
return MagicMock(
scalar_one_or_none=MagicMock(return_value=None),
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[]))),
)
mock_session = AsyncMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock()
mock_session.execute = AsyncMock(side_effect=execute_router)
mock_session.commit = AsyncMock()
return mock_session
@pytest.mark.asyncio
async def test_reprint_clears_timelapse_path_and_unlinks_stale_file(tmp_path):
"""On reprint promotion, timelapse_path must be reset to None and the old
on-disk video unlinked, so the completion-time scanner and finish-photo
extractor don't reuse the original run's frame."""
mock_printer = MagicMock()
mock_printer.id = 1
mock_printer.auto_archive = True
mock_printer.external_camera_enabled = False
mock_printer.external_camera_url = None
mock_printer.name = "TestP2S"
# Lay down a fake stale timelapse under a tmp base_dir so the unlink
# actually has a file to remove.
relpath = "archives/42/timelapse/original.mp4"
stale_file = tmp_path / relpath
stale_file.parent.mkdir(parents=True, exist_ok=True)
stale_file.write_bytes(b"old timelapse bytes")
assert stale_file.exists()
mock_archive = MagicMock()
mock_archive.id = 42
mock_archive.filename = "MyModel.3mf"
mock_archive.subtask_id = None
mock_archive.print_time_seconds = None
mock_archive.created_by_id = None
mock_archive.printer_id = 1
mock_archive.print_name = "MyModel"
mock_archive.status = "archived"
mock_archive.file_path = "archives/42/MyModel.3mf"
mock_archive.energy_start_kwh = None
mock_archive.timelapse_path = relpath # stale from the original run
register_expected_print(1, "MyModel.3mf", archive_id=42, ams_mapping=None)
mock_session = _build_mocks(mock_printer, mock_archive)
(
async_session_p,
notif_p,
plug_p,
ws_p,
pm_p,
relay_p,
_energy,
_load_obj,
_store_spoolman,
_send_start,
_list_tl,
) = _patches()
with (
async_session_p as mock_session_maker,
notif_p as mock_notif,
plug_p as mock_plug,
ws_p as mock_ws,
pm_p as mock_pm,
relay_p as mock_relay,
_energy,
_load_obj,
_store_spoolman,
_send_start,
_list_tl,
patch.object(app_settings, "base_dir", tmp_path),
):
mock_session_maker.return_value = mock_session
mock_notif.on_print_start = AsyncMock()
mock_plug.on_print_start = AsyncMock()
mock_ws.send_print_start = AsyncMock()
mock_ws.send_archive_updated = AsyncMock()
mock_relay.on_print_start = AsyncMock()
mock_pm.get_printer = MagicMock(return_value=MagicMock(name="Test", serial_number="TEST123"))
from backend.app.main import on_print_start
await on_print_start(1, {"filename": "MyModel.3mf", "subtask_name": "MyModel"})
assert mock_archive.timelapse_path is None, (
"expected-archive branch must clear timelapse_path on reprint so "
"_scan_for_timelapse_with_retries doesn't early-return and "
"_capture_finish_photo_from_timelapse doesn't extract the original "
"run's last frame (#1707)"
)
assert not stale_file.exists(), (
"old timelapse file must be unlinked at reprint promotion to avoid orphans in the archive directory"
)
@pytest.mark.asyncio
async def test_reprint_with_no_timelapse_path_is_noop(tmp_path):
"""When archive has no prior timelapse_path (first print, or already
cleared), promotion must still succeed and not raise on the unlink path."""
mock_printer = MagicMock()
mock_printer.id = 1
mock_printer.auto_archive = True
mock_printer.external_camera_enabled = False
mock_printer.external_camera_url = None
mock_printer.name = "TestP2S"
mock_archive = MagicMock()
mock_archive.id = 99
mock_archive.filename = "FreshFile.3mf"
mock_archive.subtask_id = None
mock_archive.print_time_seconds = None
mock_archive.created_by_id = None
mock_archive.printer_id = 1
mock_archive.print_name = "FreshFile"
mock_archive.status = "archived"
mock_archive.file_path = "archives/99/FreshFile.3mf"
mock_archive.energy_start_kwh = None
mock_archive.timelapse_path = None # nothing to clean up
register_expected_print(1, "FreshFile.3mf", archive_id=99, ams_mapping=None)
mock_session = _build_mocks(mock_printer, mock_archive)
(
async_session_p,
notif_p,
plug_p,
ws_p,
pm_p,
relay_p,
_energy,
_load_obj,
_store_spoolman,
_send_start,
_list_tl,
) = _patches()
with (
async_session_p as mock_session_maker,
notif_p as mock_notif,
plug_p as mock_plug,
ws_p as mock_ws,
pm_p as mock_pm,
relay_p as mock_relay,
_energy,
_load_obj,
_store_spoolman,
_send_start,
_list_tl,
patch.object(app_settings, "base_dir", tmp_path),
):
mock_session_maker.return_value = mock_session
mock_notif.on_print_start = AsyncMock()
mock_plug.on_print_start = AsyncMock()
mock_ws.send_print_start = AsyncMock()
mock_ws.send_archive_updated = AsyncMock()
mock_relay.on_print_start = AsyncMock()
mock_pm.get_printer = MagicMock(return_value=MagicMock(name="Test", serial_number="TEST123"))
from backend.app.main import on_print_start
await on_print_start(1, {"filename": "FreshFile.3mf", "subtask_name": "FreshFile"})
assert mock_archive.timelapse_path is None
assert mock_archive.status == "printing"
@pytest.mark.asyncio
async def test_reprint_with_missing_stale_file_does_not_raise(tmp_path):
"""If the stale file referenced by timelapse_path no longer exists on
disk (user deleted, archive purge, container rebuilt with bind-mount
drift), promotion must still clear the field cleanly without raising."""
mock_printer = MagicMock()
mock_printer.id = 1
mock_printer.auto_archive = True
mock_printer.external_camera_enabled = False
mock_printer.external_camera_url = None
mock_printer.name = "TestP2S"
mock_archive = MagicMock()
mock_archive.id = 7
mock_archive.filename = "Ghost.3mf"
mock_archive.subtask_id = None
mock_archive.print_time_seconds = None
mock_archive.created_by_id = None
mock_archive.printer_id = 1
mock_archive.print_name = "Ghost"
mock_archive.status = "archived"
mock_archive.file_path = "archives/7/Ghost.3mf"
mock_archive.energy_start_kwh = None
# Path points at a file that doesn't exist under tmp_path.
mock_archive.timelapse_path = "archives/7/timelapse/vanished.mp4"
register_expected_print(1, "Ghost.3mf", archive_id=7, ams_mapping=None)
mock_session = _build_mocks(mock_printer, mock_archive)
(
async_session_p,
notif_p,
plug_p,
ws_p,
pm_p,
relay_p,
_energy,
_load_obj,
_store_spoolman,
_send_start,
_list_tl,
) = _patches()
with (
async_session_p as mock_session_maker,
notif_p as mock_notif,
plug_p as mock_plug,
ws_p as mock_ws,
pm_p as mock_pm,
relay_p as mock_relay,
_energy,
_load_obj,
_store_spoolman,
_send_start,
_list_tl,
patch.object(app_settings, "base_dir", tmp_path),
):
mock_session_maker.return_value = mock_session
mock_notif.on_print_start = AsyncMock()
mock_plug.on_print_start = AsyncMock()
mock_ws.send_print_start = AsyncMock()
mock_ws.send_archive_updated = AsyncMock()
mock_relay.on_print_start = AsyncMock()
mock_pm.get_printer = MagicMock(return_value=MagicMock(name="Test", serial_number="TEST123"))
from backend.app.main import on_print_start
await on_print_start(1, {"filename": "Ghost.3mf", "subtask_name": "Ghost"})
assert mock_archive.timelapse_path is None
assert mock_archive.status == "printing"