Browse Source

Fix inconsistent print cost calculation on reprints (#505)

Three independent code paths wrote to archive.cost with conflicting
strategies, causing the same model to produce different prices on each
reprint (e.g. £0.77, £1.54, £2.03).

- Remove add_reprint_cost (Path 3) — redundant cost accumulation that
  double-counted on top of usage tracker
- Fix usage tracker (Path 2) — compute cost from current session's
  results only, not a SUM of all historical SpoolUsageHistory rows
- Remove _reprint_archives tracking set from main.py
- Update test mocks to match simplified query pattern

archive.cost now always reflects the cost of a single print.
maziggy 3 months ago
parent
commit
c2b7b6a99a

+ 2 - 1
CHANGELOG.md

@@ -2,9 +2,10 @@
 
 All notable changes to Bambuddy will be documented in this file.
 
-## [0.2.1b4] - Unreleased
+## [0.2.1] - Unreleased
 
 ### Fixed
+- **Inconsistent Print Cost on Reprints** ([#505](https://github.com/maziggy/bambuddy/issues/505)) — Reprinting the same model produced different costs each time (e.g., £0.77, £1.54, £2.03 for the same print). Three independent code paths wrote to `archive.cost` with conflicting strategies: the usage tracker summed ALL historical `SpoolUsageHistory` rows for the archive (including rows from previous reprints), and a separate `add_reprint_cost` method added yet another full print's cost on top. Removed the redundant `add_reprint_cost` path entirely and changed the usage tracker to compute cost only from the current print session's results instead of querying all historical rows. `archive.cost` now always reflects the cost of a single print.
 - **Timestamps Off by Timezone Offset in Non-UTC Docker Containers** ([#504](https://github.com/maziggy/bambuddy/issues/504)) — All backend timestamps used `datetime.now()` (server local time) or the deprecated `datetime.utcnow()`. The frontend's `parseUTCDate()` assumes timestamps without timezone indicators are UTC and appends `'Z'`, so when the container's timezone wasn't UTC, every stored timestamp was off by the timezone offset. Replaced all database and comparison timestamps with `datetime.now(timezone.utc)` across 16 backend files (~80 call sites). On the frontend, replaced 13 `new Date(backendTimestamp)` calls with `parseUTCDate()` across 8 files to correctly interpret UTC timestamps. Cosmetic timestamps (filenames, user-facing local time formatting) are intentionally left as local time.
 - **"Power Off Printer" Option Not Gated by Control Permission** ([#500](https://github.com/maziggy/bambuddy/issues/500)) — The "Power off printer when done" checkbox in the print modal and the auto power off toggle in the bulk edit modal were accessible to all users regardless of permissions. Users without the `printers:control` permission can now no longer enable auto power off — the checkbox and tri-state toggle are disabled and visually dimmed.
 - **Created Admin Users Can't See Settings Button** ([#503](https://github.com/maziggy/bambuddy/issues/503)) — The sidebar hid the Settings link based on a hardcoded `role === 'user'` check instead of the actual `settings:read` permission, so newly created admin users who had the permission still couldn't see the button. Also, after login the auth state was set directly from the login response instead of re-fetching the full auth status, which could miss permission data. Now uses `hasPermission('settings:read')` for the sidebar check and calls `checkAuthStatus()` after login to load the complete user state including permissions.

+ 1 - 1
backend/app/core/config.py

@@ -5,7 +5,7 @@ from pathlib import Path
 from pydantic_settings import BaseSettings
 
 # Application version - single source of truth
-APP_VERSION = "0.2.1b4"
+APP_VERSION = "0.2.1"
 GITHUB_REPO = "maziggy/bambuddy"
 
 # App directory - where the application is installed (for static files)

+ 0 - 16
backend/app/main.py

@@ -253,9 +253,6 @@ _expected_prints: dict[tuple[int, str], int] = {}
 # Track starting energy for prints: {archive_id: starting_kwh}
 _print_energy_start: dict[int, float] = {}
 
-# Track reprints to add costs on completion: {archive_id}
-_reprint_archives: set[int] = set()
-
 # Track AMS mapping for prints: {archive_id: [global_tray_id_per_slot]}
 # Used by usage tracker to map 3MF slots to physical AMS trays
 _print_ams_mappings: dict[int, list[int]] = {}
@@ -1270,10 +1267,6 @@ async def on_print_start(printer_id: int, data: dict):
                 if subtask_name:
                     _active_prints[(printer_id, f"{subtask_name}.3mf")] = archive.id
 
-                # Mark as reprint so we add cost on completion
-                _reprint_archives.add(archive.id)
-                logger.info("Marked archive %s as reprint for cost addition on completion", archive.id)
-
                 # Set up energy tracking
                 try:
                     plug_result = await db.execute(select(SmartPlug).where(SmartPlug.printer_id == printer_id))
@@ -2415,15 +2408,6 @@ async def on_print_complete(printer_id: int, data: dict):
                 "[ARCHIVE] Archive %s status updated to %s, failure_reason=%s", archive_id, status, failure_reason
             )
 
-            # Add cost for reprints (first prints have cost set in archive_print())
-            if status == "completed" and archive_id in _reprint_archives:
-                _reprint_archives.discard(archive_id)
-                try:
-                    await service.add_reprint_cost(archive_id)
-                    logger.info("[ARCHIVE] Added reprint cost for archive %s", archive_id)
-                except Exception as e:
-                    logger.warning("[ARCHIVE] Failed to add reprint cost for archive %s: %s", archive_id, e)
-
             await ws_manager.send_archive_updated(
                 {
                     "id": archive_id,

+ 0 - 37
backend/app/services/archive.py

@@ -992,43 +992,6 @@ class ArchiveService:
         await self.db.commit()
         return True
 
-    async def add_reprint_cost(self, archive_id: int) -> bool:
-        """Add cost for a reprint to the existing archive cost."""
-        archive = await self.get_archive(archive_id)
-        if not archive:
-            return False
-
-        if not archive.filament_used_grams or not archive.filament_type:
-            return False
-
-        # Calculate cost based on filament type or default
-        from backend.app.api.routes.settings import get_setting
-
-        primary_type = archive.filament_type.split(",")[0].strip()
-
-        # Look up filament cost_per_kg from database
-        filament_result = await self.db.execute(select(Filament).where(Filament.type == primary_type).limit(1))
-        filament = filament_result.scalar_one_or_none()
-
-        if filament:
-            cost_per_kg = filament.cost_per_kg
-        else:
-            # Use default filament cost from settings
-            default_cost_setting = await get_setting(self.db, "default_filament_cost")
-            cost_per_kg = float(default_cost_setting) if default_cost_setting else 25.0
-
-        additional_cost = round((archive.filament_used_grams / 1000) * cost_per_kg, 2)
-
-        # Add to existing cost (or set if None)
-        if archive.cost is None:
-            archive.cost = additional_cost
-        else:
-            archive.cost = round(archive.cost + additional_cost, 2)
-
-        await self.db.commit()
-        logger.info("Added reprint cost %s to archive %s, new total: %s", additional_cost, archive_id, archive.cost)
-        return True
-
     async def list_archives(
         self,
         printer_id: int | None = None,

+ 8 - 22
backend/app/services/usage_tracker.py

@@ -431,34 +431,20 @@ async def on_print_complete(
     if results:
         await db.commit()
 
-    # --- Update PrintArchive.cost to sum all SpoolUsageHistory costs for this archive ---
+    # --- Update PrintArchive.cost from THIS print session only ---
 
-    if archive_id:
-        from sqlalchemy import func, select
+    if archive_id and results:
+        from sqlalchemy import select
 
         from backend.app.models.archive import PrintArchive
 
-        # First try: sum by archive_id
-        cost_result = await db.execute(
-            select(func.coalesce(func.sum(SpoolUsageHistory.cost), 0)).where(SpoolUsageHistory.archive_id == archive_id)
-        )
-        total_cost = cost_result.scalar() or 0
-
-        # Fallback: if no cost found, sum by print_name and printer_id (legacy)
         archive_result = await db.execute(select(PrintArchive).where(PrintArchive.id == archive_id))
         archive = archive_result.scalar_one_or_none()
-        if archive and total_cost == 0 and archive.print_name and archive.printer_id:
-            legacy_cost_result = await db.execute(
-                select(func.coalesce(func.sum(SpoolUsageHistory.cost), 0)).where(
-                    SpoolUsageHistory.archive_id.is_(None),
-                    SpoolUsageHistory.print_name == archive.print_name,
-                    SpoolUsageHistory.printer_id == archive.printer_id,
-                )
-            )
-            total_cost = legacy_cost_result.scalar() or 0
-        if archive and total_cost > 0:
-            archive.cost = round(total_cost, 2)
-            await db.commit()
+        if archive:
+            total_cost = sum(r.get("cost", 0) or 0 for r in results)
+            if total_cost > 0:
+                archive.cost = round(total_cost, 2)
+                await db.commit()
 
     return results
 

+ 2 - 8
backend/tests/unit/test_cost_tracking.py

@@ -556,12 +556,8 @@ class TestCostAggregation:
         responses.append(("scalar_one_or_none", assignment))
         # 4. select(Spool) → spool
         responses.append(("scalar_one_or_none", spool))
-        # 5. cost aggregation: coalesce(sum(cost)) → 0 (no costs)
-        responses.append(("scalar", 0))
-        # 6. select(PrintArchive) → archive (for the guard check)
+        # 5. cost aggregation: select archive to update cost
         responses.append(("scalar_one_or_none", archive))
-        # 7. legacy fallback: coalesce(sum(cost)) → 0
-        responses.append(("scalar", 0))
 
         db = AsyncMock()
         call_count = [0]
@@ -646,9 +642,7 @@ class TestCostAggregation:
         responses.append(("scalar_one_or_none", None))  # queue item
         responses.append(("scalar_one_or_none", assignment))
         responses.append(("scalar_one_or_none", spool))
-        # cost aggregation: sum returns 0.50
-        responses.append(("scalar", expected_cost))
-        # select archive for guard
+        # cost aggregation: select archive to update cost
         responses.append(("scalar_one_or_none", archive))
 
         db = AsyncMock()