diff --git a/CHANGELOG.md b/CHANGELOG.md index c910a8553..8ce61cd95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to Bambuddy will be documented in this file. ## [0.2.5b1] - Unreleased ### Fixed +- **Stats: Failure Analysis widget no longer shows "Unknown" for archives classified after the fact (#1444, reported and root-caused by @needo37)** — Reporter spotted that the Stats page "Top Failure Reasons" widget grouped failed prints as `Unknown` even after they'd been classified via the Edit Archive modal. He went through the data layer and identified the desync: two `failure_reason` columns exist — `print_archives.failure_reason` written by `PATCH /archives/{id}` and `print_log_entries.failure_reason` read by the widget (`backend/app/services/failure_analysis.py:88`). `PrintLogEntry.failure_reason` gets captured exactly once at print-completion time (`backend/app/main.py:3641`) by copying `archive.failure_reason` — and at that moment the archive value is still `NULL` because the user hasn't picked a reason yet. The Edit Archive modal's PATCH route then writes only to `print_archives` via a generic `setattr` loop, never touching the log entry → widget stays stuck on `Unknown` forever. The reporter confirmed the desync at the DB level (`archive.failure_reason = 'Adhesion failure'`, `print_log_entry.failure_reason = NULL`). Fix mirrors `failure_reason` and `status` from the PATCH payload to the most recent `PrintLogEntry` for that archive (highest `id`). Latest-only because `archive.failure_reason` / `status` already reflect the *latest* run's outcome (each reprint clears the archive's reason at `main.py:2195` and rewrites it at completion), so the Edit Archive modal is implicitly showing — and editing — the latest run; reprints of an archive that succeeded on the second attempt keep the original failed run's classification intact. Scoped to those two fields only — `cost`, `print_name`, `printer_id` etc are deliberately *not* mirrored because per-run values legitimately diverge from archive-level ones (e.g. partial-print cost on a failed run differs from the source archive's full-print cost, see `_compute_run_filament_grams` at `main.py:596`). **Tests** (3 new in `test_archives_api.py`): the bug repro (failure_reason mirrors), the status case (the second field the reporter flagged), and the reprint guard (only the latest of multiple entries gets touched, an earlier entry keeps its prior reason). 55 archives-API tests green; ruff clean. + - **SpoolBuddy: spool ID surfaced everywhere a spool's identity is rendered + Write-Tag page honours Spoolman mode (#1439, reported + partially prototyped by @flom89)** — Reporter buys filament in bulk and registers every individual spool in Spoolman at intake time (each gets a unique ID + a printed barcode that goes onto the physical roll when it's unboxed). When linking an NFC tag to one of those rolls in SpoolBuddy, the picker showed only material + colour + brand — so for ten identical "Black PLA" rolls every row looked the same. The user had no way to tell which physical spool they were about to bind the tag to; the original #1385 fix had surfaced the ID in Bambuddy's main UI (SpoolFormModal, FilamentHoverCard, the inventory-mode LinkSpoolModal) but the parallel SpoolBuddy components had been missed — they ship as part of the Bambuddy frontend repo under `frontend/src/components/spoolbuddy/` and `frontend/src/pages/spoolbuddy/`, not as a separate codebase. **Part 1 — ID surface, seven spots**: `#` in muted small monospace added to `LinkSpoolModal.tsx` (the link-tag-to-spool picker — the reporter's primary use case), `SpoolBuddyWriteTagPage.tsx` (write-tag picker — reporter's second screenshot), `AssignToAmsModal.tsx` header (single-spool context but disambiguating IDs help confirm the right roll was picked), `TagDetectedModal.tsx` (defined but unmounted today — kept consistent for future use), `SpoolInfoCard.tsx` (the found-tag panel on the right side of the dashboard — the "main screen" view), `InventorySpoolInfoCard.tsx` (matching inventory variant), and `SpoolBuddyAmsPage.tsx` AMS-slot assigned-spool block. All seven placements mirror the #1385 pattern (`#` with `shrink-0` so truncation never hides the ID). Frontend-only — the ID was already on the Spool / InventorySpool API shape (`spool.id`); these seven files just weren't surfacing it. **Part 2 — Spoolman-mode parity on the Write-Tag page**: reporter then surfaced that the same page hardcoded `api.getSpools(false)` regardless of inventory backend — so users in Spoolman mode (whose authoritative inventory is at Spoolman, not the internal table) saw spools they never created, and a successful tag write would bind the NFC tag to the wrong backend (the backend `/spoolbuddy/nfc/write-tag` route is mode-aware via `_get_spoolman_client_or_none`, but the frontend was driving it with internal-mode IDs that don't exist on the Spoolman side). Fix follows the wrapper pattern InventoryPage uses (`InventoryPageRouter` at `:445`): page detects `spoolmanMode` from a `getSpoolmanSettings` query at the top and threads it through, with `enabled: spoolmanModeReady` gating the spool fetch until settings load so we don't burn a wrong-backend request during the initial render. Every API call in the page now branches on `spoolmanMode` — 6 sites: the main spool list, the NewSpoolTouchForm's autocomplete spool list, the untag flow (`linkTagToSpool` vs `linkTagToSpoolmanSpool` — the Spoolman variant doesn't accept `data_origin` since Spoolman manages that), the K-profile save (`saveSpoolKProfiles` vs `saveSpoolmanKProfiles`), single-spool create (`createSpool` vs `createSpoolmanInventorySpool`), and bulk create (`bulkCreateSpools` vs `bulkCreateSpoolmanInventorySpools` — the Spoolman variant returns a `SpoolmanBulkCreateResult` envelope vs raw array, handled with a duck-typed `'created' in result` check that mirrors `SpoolFormModal`'s existing pattern). Same shape rule as [[feedback_sqlite_and_postgres_upfront]] / [[feedback_inventory_modes_parity]]: both modes ship in the same drop, no `spoolmanMode ? undefined : ...` UI gates. **Tests**: 3 new in `SpoolBuddyWriteTagPage.test.tsx` — the ID-visibility regression with two identical PLA-Red rolls IDs 42 / 43 (a future refactor that drops the ID span breaks it), plus two parity regressions (`reads from internal inventory when Spoolman mode is OFF` and `reads from Spoolman when Spoolman mode is ON` — the latter asserts `getSpools` is NOT called when the user is in Spoolman mode, so re-hardcoding the internal endpoint breaks CI immediately). 11 WriteTagPage tests + 51 other SpoolBuddy component tests green (62 total); frontend build clean. - **FTP: P2S upload truncates / 426 "Failure reading network stream" on Python 3.13 (#1401, reported and root-caused by @iitazz)** — Reporter on a P2S running firmware 01.02.00.00 saw every Bambuddy-initiated print fail with the printer's on-screen "unable to parse 3mf file" error ~30 s in; downloading the file back off the printer's SD card confirmed it was truncated at exactly 7 × 64 KB (clean chunk-boundary cut). Initial #1417 follow-up tightened our 426 handling so we'd surface upload failures instead of silently dispatching a print of a partial 3MF — but that only stopped Bambuddy from hiding the problem; the actual upload still failed. The reporter then dug into it with Gemini and identified the real cause: Python 3.13's default `ssl.create_default_context()` negotiates TLS 1.3 when both peers support it, but the printer's vsFTPd build implements session reuse on the FTPS data channel against an old OpenSSL that doesn't tolerate TLS 1.3's asynchronous session-ticket model. The control-channel handshake completes, the data channel tries to resume the session, the resumption races, the data channel gets torn down mid-stream — first ~448 KB of bytes already in the TCP buffer land on the SD card, the rest never make it, printer's vsFTPd replies 426 instead of 226. **Fix** caps the SSL context's `maximum_version` to TLS 1.2 so session resumption is synchronous and the upload completes normally. Implementation follows the pattern just established by `camera_profiles.py` in the #1395 follow-up: a new `backend/app/services/ftp_profiles.py` module with an `FTPProfile` frozen dataclass (one field today, `cap_tls_v1_2: bool = False`) and a per-model registry. Default profile keeps the historical TLS-1.3 negotiation; P2S (display name + internal SSDP code N7) overrides with `cap_tls_v1_2=True`. `ImplicitFTP_TLS.__init__` gains a matching `cap_tls_v1_2` kwarg; `BambuFTPClient.connect()` looks up the profile and threads the flag through. **Deliberately scoped to P2S only** — X1C / P1S / H2D installs that work today stay on the negotiated TLS 1.3; flipping a future model to the capped path is a one-line entry in `_PROFILES` when a new reporter surfaces the same symptom. Considered but rejected the reporter's second proposed change (revert manual `transfercmd` + `sendall` back to `storbinary`) — the stated rationale ("raw sendall breaks OpenSSL 3.x framing") is incorrect (CPython's `storbinary` itself uses `sendall` internally; the actual socket-level behaviour is identical), the move to manual `transfercmd` was deliberate to dodge A1 hanging in `storbinary`'s synchronous `voidresp()`, and the #1417 SIZE-check escape for the "data is intact on the SD card despite the 426" race lives in the manual-transfer path — a switch to `storbinary` would lose that protection. **Tests**: 9 new in `test_ftp_profiles.py` (default profile doesn't cap; unknown / empty model falls back; P2S display name and N7 SSDP code both resolve to capped; lookup is case-insensitive; X1C / H2D / P1S / A1 stay uncapped; dataclass is frozen; **integration test pins the wiring** — `ImplicitFTP_TLS(cap_tls_v1_2=True)` actually sets `ssl_context.maximum_version == TLSVersion.TLSv1_2`, guards against a future refactor that drops the profile→context wiring while keeping the registry looking correct). 87 existing `test_bambu_ftp.py` tests still green; ruff clean. diff --git a/backend/app/api/routes/archives.py b/backend/app/api/routes/archives.py index 266cd699e..227afd4b7 100644 --- a/backend/app/api/routes/archives.py +++ b/backend/app/api/routes/archives.py @@ -1343,9 +1343,35 @@ async def update_archive( if archive.created_by_id != user.id: raise HTTPException(403, "You can only update your own archives") - for field, value in update_data.model_dump(exclude_unset=True).items(): + update_payload = update_data.model_dump(exclude_unset=True) + for field, value in update_payload.items(): setattr(archive, field, value) + # #1444: Mirror per-run classification fields to the most recent + # PrintLogEntry for this archive. PrintLogEntry.failure_reason is captured + # once at print-completion time from archive.failure_reason — which is + # NULL until the user classifies the failure via the Edit Archive modal. + # Without this mirror the Failure Analysis widget (which groups by + # print_log_entries.failure_reason) keeps showing "Unknown" forever. + # Same desync hits status: flipping it in the modal wouldn't update the + # entry either. Only the latest entry is touched because that's the run + # the modal is implicitly showing (archive.failure_reason / status are + # overwritten on each reprint to reflect the latest run's outcome). + mirror_fields = {"failure_reason", "status"} + to_mirror = {k: v for k, v in update_payload.items() if k in mirror_fields} + if to_mirror: + from backend.app.models.print_log import PrintLogEntry + + latest_entry = await db.scalar( + select(PrintLogEntry) + .where(PrintLogEntry.archive_id == archive_id) + .order_by(PrintLogEntry.id.desc()) + .limit(1) + ) + if latest_entry is not None: + for field, value in to_mirror.items(): + setattr(latest_entry, field, value) + await db.commit() # Re-fetch with relationships loaded after commit diff --git a/backend/tests/integration/test_archives_api.py b/backend/tests/integration/test_archives_api.py index 8f870b34a..c33fa8169 100644 --- a/backend/tests/integration/test_archives_api.py +++ b/backend/tests/integration/test_archives_api.py @@ -168,6 +168,102 @@ class TestArchivesAPI: assert response.status_code == 200 assert response.json()["external_url"] is None + @pytest.mark.asyncio + @pytest.mark.integration + async def test_update_archive_failure_reason_mirrors_to_print_log_entry( + self, async_client: AsyncClient, archive_factory, printer_factory, db_session + ): + """#1444: PATCH /archives/{id} with failure_reason must mirror to the + latest PrintLogEntry so the Stats page Failure Analysis widget + (which reads PrintLogEntry.failure_reason) reflects the user's + reclassification instead of showing "Unknown" forever. + """ + from sqlalchemy import select + + from backend.app.models.print_log import PrintLogEntry + + printer = await printer_factory() + # archive_factory auto-creates a matching PrintLogEntry (failure_reason + # carried from the archive, which is NULL here — same shape as the bug + # repro: print completed → log entry written with NULL → user goes to + # classify the failure afterwards). + archive = await archive_factory(printer.id, print_name="Failed Print", status="failed", run_status="failed") + + response = await async_client.patch( + f"/api/v1/archives/{archive.id}", + json={"failure_reason": "Adhesion failure"}, + ) + assert response.status_code == 200 + + result = await db_session.execute(select(PrintLogEntry).where(PrintLogEntry.archive_id == archive.id)) + mirrored = result.scalar_one() + assert mirrored.failure_reason == "Adhesion failure" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_update_archive_status_mirrors_to_print_log_entry( + self, async_client: AsyncClient, archive_factory, printer_factory, db_session + ): + """#1444: PATCH /archives/{id} with status must mirror to the latest + PrintLogEntry so stats that filter on PrintLogEntry.status see the + user's reclassification. + """ + from sqlalchemy import select + + from backend.app.models.print_log import PrintLogEntry + + printer = await printer_factory() + archive = await archive_factory(printer.id, run_status="completed") + + response = await async_client.patch( + f"/api/v1/archives/{archive.id}", + json={"status": "failed"}, + ) + assert response.status_code == 200 + + result = await db_session.execute(select(PrintLogEntry).where(PrintLogEntry.archive_id == archive.id)) + mirrored = result.scalar_one() + assert mirrored.status == "failed" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_update_archive_failure_reason_only_touches_latest_entry( + self, async_client: AsyncClient, archive_factory, printer_factory, db_session + ): + """#1444: For an archive with multiple runs (reprints), only the + latest PrintLogEntry should receive the reclassification. Earlier + runs were classified at their own time and must not be retroactively + overwritten. + """ + from backend.app.models.print_log import PrintLogEntry + + printer = await printer_factory() + # First run — created by the factory's auto-run with its own reason. + archive = await archive_factory(printer.id, status="failed", run_status="failed") + from sqlalchemy import select + + first_run = ( + await db_session.execute(select(PrintLogEntry).where(PrintLogEntry.archive_id == archive.id)) + ).scalar_one() + first_run.failure_reason = "Filament tangle" + await db_session.commit() + + # Second run — the reprint that just finished with NULL classification. + latest_run = PrintLogEntry(archive_id=archive.id, status="failed", failure_reason=None) + db_session.add(latest_run) + await db_session.commit() + + response = await async_client.patch( + f"/api/v1/archives/{archive.id}", + json={"failure_reason": "Adhesion failure"}, + ) + assert response.status_code == 200 + + await db_session.refresh(first_run) + await db_session.refresh(latest_run) + assert first_run.failure_reason == "Filament tangle" + assert latest_run.failure_reason == "Adhesion failure" + # ======================================================================== # Delete endpoints # ========================================================================