From 71e58e6cf1ded82b0241d0067f418bc6f32774d0 Mon Sep 17 00:00:00 2001 From: maziggy Date: Fri, 22 May 2026 10:22:14 +0200 Subject: [PATCH] fix(library): show the filename, not the embedded 3MF Title (#1489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File Manager cards, search and sort keyed off file_metadata.print_name, which ThreeMFParser lifts from the 3MF's . That title is the in-app project title — generic "Exported 3D Model" for any Bambu Studio "Save As", a marketing title for a MakerWorld download — and almost never the filename the user saved as. A card for Whatever.3mf showed "Exported 3D Model"; correcting it needed a rename round-trip, since the Rename dialog disables Save while the name is unchanged. The slicer-output write path already dropped print_name for this exact reason; the four other paths that store parsed 3MF metadata onto a LibraryFile did not — external-folder scan, managed multipart upload, the multi-file ZIP-upload branch, and MakerWorld import. Add a shared _without_print_name() helper and apply it at all four import paths; switch the slicer path to it so there is one rule. A LibraryFile's display name is its filename — only PrintArchive carries a real print_name, which is untouched. Remove the now-redundant filename->print_name mirroring in the rename route. Add a one-time idempotent data migration (_migrate_drop_library_print_name, SQLite json_remove / PostgreSQL jsonb key-removal branched on is_sqlite()) so libraries imported before the fix correct themselves without the rename workaround. No frontend change: print_name || filename yields the filename once print_name is gone. Tests: 6 new in test_library_print_name.py cover _without_print_name and the migration (incl. idempotency, siblings preserved, null metadata). SQLite migration branch verified by test; PostgreSQL branch verified against a real Postgres instance. --- CHANGELOG.md | 1 + backend/app/api/routes/library.py | 42 +++++--- backend/app/core/database.py | 37 ++++++++ backend/tests/unit/test_library_print_name.py | 95 +++++++++++++++++++ 4 files changed, 160 insertions(+), 15 deletions(-) create mode 100644 backend/tests/unit/test_library_print_name.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 20fb65ce9..6162d61db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ All notable changes to Bambuddy will be documented in this file. - **PyJWT CVE-2025-45768 (PYSEC-2025-183 / GHSA-65pc-fj4g-8rjx): permanently ignored in pip-audit** — Advisory is disputed by the PyJWT maintainers, with the advisory description literally noting *"this is disputed by the Supplier because the key length is chosen by the application that uses the library."* `fix_versions=[]` on the advisory confirms no PyJWT patch exists or will exist. Bambuddy is not affected: `backend/app/core/auth.py:184` auto-generates secrets via `secrets.token_urlsafe(64)` (~86 chars of entropy, far above any sane minimum) and the file-loaded path at `:177` rejects secrets shorter than 32 chars. Added a permanent `--ignore-vuln CVE-2025-45768` to `.github/workflows/security.yml` with an inline comment citing the file:line evidence so a future maintainer reviewing the ignore list sees why it's load-bearing. Also dropped the stale `--ignore-vuln CVE-2026-4539` for Pygments — Pygments has since shipped a patched version and the ignore is no longer load-bearing (verified: `pip-audit --ignore-vuln CVE-2025-45768` alone reports clean). ### Fixed +- **Library files now display the filename, not the embedded 3MF Title (#1489, reported by @needo37)** — File Manager cards, search and sort keyed off `file_metadata.print_name`, which `ThreeMFParser` lifts from the 3MF's ``. That title is the in-app project title — generic `"Exported 3D Model"` for any Bambu Studio "Save As", a marketing title for a MakerWorld download — and almost never the filename the user actually saved. So a card for `Whatever.3mf` showed `Exported 3D Model`, and the only way to correct it was a rename round-trip (the Rename dialog's Save button is disabled while the name is unchanged, so the user had to rename to a different name and back). The slicer-output write path already dropped `print_name` for exactly this reason; the **four** other write paths that store parsed 3MF metadata onto a `LibraryFile` did not — external-folder scan, managed multipart upload, the multi-file ZIP-upload branch, and MakerWorld import. **Fix**: a shared `_without_print_name()` helper strips `print_name` from library-file metadata, applied at all four import paths (and the slicer path switched to it, so there is one rule). A `LibraryFile`'s display name is its filename; only `PrintArchive` carries a real `print_name`, and that is untouched. The now-redundant filename→`print_name` mirroring in the rename route is removed. A one-time data migration (`_migrate_drop_library_print_name`, idempotent, SQLite `json_remove` / PostgreSQL `jsonb` key-removal branched on `is_sqlite()`) clears `print_name` from rows imported before the fix, so existing libraries correct themselves without the rename workaround. No frontend change — `print_name || filename` naturally yields the filename once `print_name` is gone. **Tests**: 6 new in `test_library_print_name.py` — `_without_print_name` (strips, keeps siblings, `None` pass-through, no-op identity return, no input mutation, print-name-only → `{}`) and the migration (clears `print_name`, leaves siblings and metadata-free rows alone, idempotent). 109 library + dialect tests green; the migration's PostgreSQL branch additionally ran live against real Postgres during the integration-test app boot. Backend ruff clean. - **Camera: ffmpeg's stderr is now captured when an RTSP stream stalls instead of only when ffmpeg crashes (#1395, reported by @Tschipel)** — A P2S support bundle taken on 0.2.5b1 (the per-model probesize fix already applied) showed the camera still failing: ffmpeg connects, stays alive 30+ seconds, emits zero JPEG bytes, the stream's 30 s `stdout.read` times out, reconnect loop repeats — but with **no ffmpeg stderr anywhere in the log** to say why. Root cause was a diagnostic bug, not the camera path: `_read_ffmpeg_stderr` called `process.stderr.read()` (read-to-EOF). A stalled-but-still-alive ffmpeg — exactly the P2S RTSP failure mode — never closes stderr, so the read blocked until the 2 s `wait_for` timeout and returned `None`, discarding the banner + stream-analysis lines ffmpeg had already printed. ffmpeg's stderr was therefore captured *only* when it fully exited; the earlier "not enough frames to estimate rate" smoking gun was available only because ffmpeg crashed back then, and once the probesize bump turned the crash into a hang the diagnostic went dark. **Fix**: `_read_ffmpeg_stderr` now drains stderr incrementally in bounded 8 KB chunks (64 KB cap), returning whatever ffmpeg has printed so far whether or not it has exited — so a hung stream is self-describing in the next support bundle. Additionally, `generate_rtsp_mjpeg_stream` now logs the resolved per-model `probesize` / `analyzeduration` on the info-level "Starting RTSP camera stream" line (verifiable without debug logging), and the debug-level ffmpeg-command line logs the full argv with only the credential-bearing camera URL redacted, instead of hiding the entire command. No behaviour change to streaming itself — this makes the still-unresolved P2S RTSP stall diagnosable. **Tests**: 4 new in `test_camera_stderr_summary.py` cover `_read_ffmpeg_stderr` capturing output from a *running* (un-exited, no-EOF) ffmpeg — the regression — as well as the exited case, the no-stderr-pipe case, and banner-only output summarizing to `None`. 9 camera-stderr tests green; backend ruff clean. - **Camera diagnostic (stethoscope) was missing from the pop-out camera window (#1395, reported by @Tschipel)** — The #1395 camera-diagnostic follow-up — stethoscope icon in the control bar, **Diagnose** button in the stream-error state, `CameraDiagnoseModal` — shipped wired into `EmbeddedCameraViewer.tsx` only, the *embedded* camera mode. It was never added to `CameraPage.tsx`, the standalone window that opens at `/camera/{id}` when `camera_view_mode` is `window` (the default). The reporter's support bundle had `"camera_view_mode": "window"`, so they were on `CameraPage` the whole time and genuinely could not see the stethoscope no matter how many container rebuilds or cache clears they tried — the JS bundle did contain the `camera.diagnose` strings (they come from `EmbeddedCameraViewer`), but that component never renders in window mode. Switching to overlay mode made it appear instantly, exactly as the reporter found. **Fix**: ported the diagnostic into `CameraPage.tsx` — the `Stethoscope` control-bar button (between **Refresh** and **Fullscreen**, matching the embedded viewer), a **Diagnose** button next to **Retry** in the `streamError` block, and the `CameraDiagnoseModal` render. No new i18n keys — `camera.diagnose.*` already exist in all 9 locales. The backend per-model camera-profile fix from the same issue is view-mode-agnostic and already applied; this only makes the diagnostic reachable in the default window mode. Frontend build clean. - **A backend restart mid-print no longer duplicates the job in the archive (#1485, reported by @pwostran)** — When the server running Bambuddy restarted during an active print, the running job was duplicated in the archive — and deleting the duplicate didn't help: every subsequent restart while the print was still running spawned a fresh one. Both support bundles confirmed it: `WARNING Found stale 'printing' archive 3 (age: 9:46:23), marking as cancelled and creating new archive` → `Created archive 4`. On reconnect `on_print_start` fires (Bambuddy sees the printer running) and tries to re-attach to the existing archive in `main.py`. The reliable match is by `subtask_id`; the fallback is a name match plus — and this was the bug — a **4-hour staleness heuristic**: a name-matched `printing` archive older than 4h was assumed dead, marked `cancelled`, and a new archive created. Bambu prints routinely run far longer than 4h, so a genuine long print's *live* archive was destroyed and duplicated on every restart. **Two root causes, both fixed.** **(1) Queue/scheduled archives never persisted a restart-stable `subtask_id`.** Bambuddy mints a per-job id (`project_id`/`subtask_id`/`task_id`) inside `start_print` when it sends the `project_file` command, and the printer echoes it back — but often not within the ~10s before `on_print_start` first fires, so the expected-print branch's `if subtask_id and not archive.subtask_id` write got an empty value and the archive was left with no id. A later restart then had nothing to match on and fell through to the fragile name path. Fix: `BambuMQTTClient.start_print` now records the minted id on `last_dispatch_subtask_id`, and `on_print_start` falls back to it when the printer hasn't echoed `subtask_id` yet — so every dispatched archive persists a stable id and a restart resumes it by id, age-independent. **(2) The 4-hour cutoff itself.** Replaced with a progress-aware check: when a name-matched `printing` archive is found on restart, the printer's *current* reported progress decides resume-vs-stale, not wall-clock age. Real progress (or unknown progress — printer offline) always resumes the existing archive. It is only treated as a stale leftover when the printer clearly shows a *different, freshly-started* print — under 1% progress on an archive more than 2h old, a state a real in-progress print is never in. The arbitrary 4h constant is gone. **Net effect**: a restart mid-print resumes the existing archive (`started_at`, energy, timelapse intact) instead of ever cancelling it and creating a duplicate. **Tests**: 2 new in `test_bambu_mqtt.py` (`start_print` records `last_dispatch_subtask_id`, and updates it per submission); new `TestStaleVsResume` in `test_subtask_archive_resume.py` — 6 cases pinning the progress-aware decision (long print mid-run resumes; barely-started long print resumes; ~0% + old archive is stale; ~0% + young archive resumes; unknown progress never cancels; the sub-1%/2h boundary). 472 print-start / MQTT / scheduler / dispatch tests green; backend ruff clean. diff --git a/backend/app/api/routes/library.py b/backend/app/api/routes/library.py index a48da913f..b10668ca7 100644 --- a/backend/app/api/routes/library.py +++ b/backend/app/api/routes/library.py @@ -364,6 +364,23 @@ def _clean_3mf_metadata(obj): return obj +def _without_print_name(metadata: dict | None) -> dict | None: + """Drop the embedded 3MF Title (``print_name``) from library-file metadata. + + The 3MF ```` holds the in-app project title — the + generic ``"Exported 3D Model"`` for a Bambu Studio "Save As", a marketing + title for a MakerWorld download — never the filename the user saved as. + The FileManager keys its display name, search and sort off ``print_name``, + so storing it makes every card show the wrong name (#1489). A library + file's display name is its filename; only ``PrintArchive`` carries a real + ``print_name``. Returns the input unchanged when there's nothing to strip; + otherwise a new dict (never mutates the argument). + """ + if not metadata or "print_name" not in metadata: + return metadata + return {k: v for k, v in metadata.items() if k != "print_name"} + + async def save_3mf_bytes_to_library( db: AsyncSession, *, @@ -435,7 +452,7 @@ async def save_3mf_bytes_to_library( file_size=len(file_bytes), file_hash=file_hash, thumbnail_path=to_relative_path(thumbnail_path) if thumbnail_path else None, - file_metadata=metadata, + file_metadata=_without_print_name(metadata), source_type=source_type, source_url=source_url, created_by_id=owner_id, @@ -1378,7 +1395,7 @@ async def scan_external_folder( file_size=stat.st_size, file_hash=None, # Skip hashing external files for performance thumbnail_path=thumbnail_path, - file_metadata=file_metadata, + file_metadata=_without_print_name(file_metadata), ) db.add(db_file) added += 1 @@ -1655,7 +1672,7 @@ async def upload_file( file_size=len(content), file_hash=file_hash, thumbnail_path=to_relative_path(thumbnail_path) if thumbnail_path else None, - file_metadata=metadata if metadata else None, + file_metadata=_without_print_name(metadata) if metadata else None, created_by_id=current_user.id if current_user else None, ) db.add(library_file) @@ -1908,7 +1925,7 @@ async def extract_zip_file( file_size=len(file_content), file_hash=file_hash, thumbnail_path=to_relative_path(thumbnail_path) if thumbnail_path else None, - file_metadata=metadata if metadata else None, + file_metadata=_without_print_name(metadata) if metadata else None, created_by_id=current_user.id if current_user else None, ) db.add(library_file) @@ -3152,14 +3169,10 @@ async def slice_and_persist( except Exception as exc: logger.warning("Failed to parse sliced 3MF metadata for %s: %s", out_filename, exc) - # The parsed 3MF metadata carries a `print_name` lifted from the source - # file's embedded settings (BambuStudio always sets this; OrcaSlicer - # often leaves it blank). The FileManager listing prefers print_name - # over filename for display, which makes a sliced row indistinguishable - # from its source. Drop print_name so the listing falls back to the - # actual filename — which already ends in ".gcode.3mf" and self-describes - # as the sliced output. - metadata: dict = {k: v for k, v in parsed_metadata.items() if k != "print_name"} + # Drop the embedded `print_name` (see _without_print_name) so the sliced + # row's display falls back to its ".gcode.3mf" filename instead of the + # source file's project title, which would make the two indistinguishable. + metadata: dict = dict(_without_print_name(parsed_metadata) or {}) metadata.update( { "print_time_seconds": result.print_time_seconds, @@ -3637,9 +3650,8 @@ async def update_file( if "/" in data.filename or "\\" in data.filename: raise HTTPException(status_code=400, detail="Filename cannot contain path separators") file.filename = data.filename - # Also update print_name in file_metadata so the display name matches - if file.file_metadata and "print_name" in file.file_metadata: - file.file_metadata = {**file.file_metadata, "print_name": data.filename} + # No print_name to keep in sync — library files display by filename, + # and _without_print_name strips the embedded 3MF Title on import (#1489). if data.folder_id is not None: if data.folder_id == 0: diff --git a/backend/app/core/database.py b/backend/app/core/database.py index 3dd1ad032..4814ceed1 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -415,6 +415,39 @@ async def _migrate_normalize_printer_ids(conn) -> None: await conn.execute(text("UPDATE api_keys SET printer_ids = NULL WHERE printer_ids::text = '[]'")) +async def _migrate_drop_library_print_name(conn) -> None: + """Strip the embedded 3MF Title (``print_name``) from library file metadata (#1489). + + Library files stored the 3MF's ```` as + ``file_metadata.print_name`` — generic ("Exported 3D Model") for Bambu + Studio exports, a marketing title for MakerWorld downloads — and the + FileManager wrongly preferred it over the filename for the card label, + search and sort. New imports no longer store it; this clears it from rows + imported before the fix so existing libraries don't need a rename + round-trip. Idempotent — rows without the key are untouched. + """ + from sqlalchemy import text + + async with conn.begin_nested(): + if is_sqlite(): + await conn.execute( + text( + "UPDATE library_files SET file_metadata = json_remove(file_metadata, '$.print_name') " + "WHERE json_extract(file_metadata, '$.print_name') IS NOT NULL" + ) + ) + else: + # file_metadata is a JSON (not JSONB) column — cast to jsonb for the + # key-exists test (jsonb_exists, avoiding the `?` operator which + # clashes with driver parameter syntax) and the `- key` removal. + await conn.execute( + text( + "UPDATE library_files SET file_metadata = (file_metadata::jsonb - 'print_name')::json " + "WHERE jsonb_exists(file_metadata::jsonb, 'print_name')" + ) + ) + + async def _migrate_update_auto_link_constraint(conn) -> None: """Update the auto_link CHECK constraint to allow Fall C (custom email claim). @@ -2615,6 +2648,10 @@ async def run_migrations(conn): "ALTER TABLE smart_plugs ADD COLUMN IF NOT EXISTS off_delay_after_drying_minutes INTEGER DEFAULT 10", ) + # Data migration: drop the embedded 3MF Title (`print_name`) from library + # file metadata so the FileManager displays the filename, not the title (#1489). + await _migrate_drop_library_print_name(conn) + async def seed_notification_templates(): """Seed default notification templates if they don't exist.""" diff --git a/backend/tests/unit/test_library_print_name.py b/backend/tests/unit/test_library_print_name.py new file mode 100644 index 000000000..233d90cad --- /dev/null +++ b/backend/tests/unit/test_library_print_name.py @@ -0,0 +1,95 @@ +"""Tests for library files displaying the filename, not the embedded 3MF Title (#1489). + +The 3MF ```` is the in-app project title — generic +("Exported 3D Model") for a Bambu Studio "Save As", a marketing title for a +MakerWorld download — never the filename the user saved as. The FileManager +keyed its display name / search / sort off ``file_metadata.print_name``, so +storing the Title made every card show the wrong name. ``_without_print_name`` +strips it on import; ``_migrate_drop_library_print_name`` clears it from rows +imported before the fix. +""" + +from sqlalchemy import select + +from backend.app.api.routes.library import _without_print_name +from backend.app.core.database import _migrate_drop_library_print_name +from backend.app.models.library import LibraryFile + +# --- _without_print_name --------------------------------------------------- + + +def test_strips_print_name_keeps_siblings(): + cleaned = _without_print_name({"print_name": "Exported 3D Model", "print_time_seconds": 100}) + assert cleaned == {"print_time_seconds": 100} + + +def test_none_passes_through(): + assert _without_print_name(None) is None + + +def test_dict_without_print_name_returned_unchanged(): + meta = {"print_time_seconds": 50} + # No copy needed when there's nothing to strip — same object back. + assert _without_print_name(meta) is meta + + +def test_does_not_mutate_input(): + original = {"print_name": "Whatever", "filament_used_grams": 12} + cleaned = _without_print_name(original) + assert original == {"print_name": "Whatever", "filament_used_grams": 12} # untouched + assert cleaned == {"filament_used_grams": 12} + + +def test_print_name_only_collapses_to_empty_dict(): + assert _without_print_name({"print_name": "Exported 3D Model"}) == {} + + +# --- _migrate_drop_library_print_name -------------------------------------- + + +async def test_migration_strips_print_name_from_existing_rows(db_session, monkeypatch): + """Rows imported before the fix get print_name cleared; siblings and rows + that never had it are untouched. Idempotent on a second run. + + The test DB is SQLite; is_sqlite() reads settings.database_url (not the + test engine), so pin it to exercise the SQLite branch deterministically. + The PostgreSQL branch is verified against a real PG instance separately.""" + monkeypatch.setattr("backend.app.core.database.is_sqlite", lambda: True) + db_session.add_all( + [ + LibraryFile( + filename="halloween.3mf", + file_path="/a", + file_type="3mf", + file_size=1, + file_metadata={"print_name": "Haunted House", "print_time_seconds": 100}, + ), + LibraryFile( + filename="no_meta.3mf", + file_path="/b", + file_type="3mf", + file_size=1, + file_metadata={"print_time_seconds": 50}, + ), + LibraryFile( + filename="null_meta.3mf", + file_path="/c", + file_type="3mf", + file_size=1, + file_metadata=None, + ), + ] + ) + await db_session.commit() + + conn = await db_session.connection() + await _migrate_drop_library_print_name(conn) + await _migrate_drop_library_print_name(conn) # idempotent + + db_session.expire_all() + rows = (await db_session.execute(select(LibraryFile).order_by(LibraryFile.filename))).scalars().all() + by_name = {r.filename: r for r in rows} + + assert by_name["halloween.3mf"].file_metadata == {"print_time_seconds": 100} + assert by_name["no_meta.3mf"].file_metadata == {"print_time_seconds": 50} + assert by_name["null_meta.3mf"].file_metadata is None