Browse Source

Keep the printer's name on statistics after it is deleted (issue #2873)

    Every per-printer breakdown resolved the name against the printers that
    exist now, so deleting a printer and choosing to keep its prints turned
    "Ultron" into "Printer 1" in Prints by Printer, the success-rate and
    time-accuracy lists, and Failures by Printer. Archives lose their printer
    on that delete as well, so nothing was left to read a name from.

    The runs themselves recorded the name they printed on. /archives/stats now
    reports the last name each id was known by - taken from the newest run that
    has one, so a later name-less row cannot blank it - and failure analysis
    falls back to the same thing for ids with no printer left. The client keeps
    preferring a live printer's own record, so a rename still shows up straight
    away rather than after the next print.
maziggy 2 weeks ago
parent
commit
dbbcf12619

+ 25 - 0
backend/app/api/routes/archives.py

@@ -1177,6 +1177,30 @@ async def get_archive_stats(
     )
     prints_by_printer = {str(k): v for k, v in printer_result.all()}
 
+    # Names for printers the client can no longer look up. The breakdowns above
+    # key on the id each run recorded, and deleting a printer while keeping its
+    # history leaves that id pointing at nothing, so a chart that used to read
+    # "Ultron" fell back to "Printer 1" (#2873). Every run also stored the name
+    # it printed on, so the last one recorded is what that id was called. The
+    # client still prefers a live printer's current name, which keeps a rename
+    # showing up straight away.
+    last_named_run = (
+        select(func.max(PrintLogEntry.id).label("entry_id"))
+        .where(
+            PrintLogEntry.printer_id.isnot(None),
+            PrintLogEntry.printer_name.isnot(None),
+            *base_conditions,
+        )
+        .group_by(PrintLogEntry.printer_id)
+        .subquery()
+    )
+    name_result = await db.execute(
+        select(PrintLogEntry.printer_id, PrintLogEntry.printer_name).join(
+            last_named_run, PrintLogEntry.id == last_named_run.c.entry_id
+        )
+    )
+    printer_names = {str(printer_id): name for printer_id, name in name_result.all()}
+
     # Time accuracy — compare each completed run's actual duration to the
     # slicer's estimate on the linked archive. Runs without a linked archive
     # (NULL archive_id) or without an estimate are excluded.
@@ -1276,6 +1300,7 @@ async def get_archive_stats(
         total_cost=round(total_cost, 2),
         prints_by_filament_type=prints_by_filament,
         prints_by_printer=prints_by_printer,
+        printer_names=printer_names,
         average_time_accuracy=average_accuracy,
         time_accuracy_by_printer=accuracy_by_printer if accuracy_by_printer else None,
         total_energy_kwh=round(total_energy_kwh, 3),

+ 4 - 0
backend/app/schemas/archive.py

@@ -173,6 +173,10 @@ class ArchiveStats(BaseModel):
     total_cost: float
     prints_by_filament_type: dict
     prints_by_printer: dict
+    # Name each printer id was last recorded under in the print log. Lets the
+    # client keep labelling history that belongs to a deleted printer (#2873);
+    # a printer that still exists is named from the live record instead.
+    printer_names: dict[str, str] = {}
     # Time accuracy stats
     # Average across all prints with data
     average_time_accuracy: float | None = None

+ 19 - 0
backend/app/services/failure_analysis.py

@@ -151,6 +151,25 @@ class FailureAnalysisService:
                 select(Printer.id, Printer.name).where(Printer.id.in_(failures_by_printer_id.keys()))
             )
             printer_names = {row[0]: row[1] for row in printers_result.fetchall()}
+            # A printer deleted with its history kept has no row left to read a
+            # name from, and "Printer 3" tells nobody which machine kept failing
+            # (#2873). Each run recorded the name it printed on, so fall back to
+            # the last one that id was known by.
+            missing = [pid for pid in failures_by_printer_id if pid not in printer_names]
+            if missing:
+                last_named_run = (
+                    select(func.max(PrintLogEntry.id).label("entry_id"))
+                    .where(PrintLogEntry.printer_id.in_(missing), PrintLogEntry.printer_name.isnot(None))
+                    .group_by(PrintLogEntry.printer_id)
+                    .subquery()
+                )
+                historic_result = await self.db.execute(
+                    select(PrintLogEntry.printer_id, PrintLogEntry.printer_name).join(
+                        last_named_run, PrintLogEntry.id == last_named_run.c.entry_id
+                    )
+                )
+                for pid, name in historic_result.fetchall():
+                    printer_names[pid] = name
             failures_by_printer = {
                 printer_names.get(pid, f"Printer {pid}"): count for pid, count in failures_by_printer_id.items()
             }

+ 112 - 0
backend/tests/integration/test_deleted_printer_keeps_name_2873.py

@@ -0,0 +1,112 @@
+"""Statistics keeps naming a printer that was deleted with its history (#2873).
+
+Deleting a printer while keeping its prints leaves the log rows pointing at an
+id nothing resolves any more, so every per-printer breakdown fell back to
+"Printer 1" for machines the reporter knew as "Ultron". Each run recorded the
+name it printed on, so that is what the aggregates report now.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime, timedelta
+
+import pytest
+from httpx import AsyncClient
+
+from backend.app.models.print_log import PrintLogEntry
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_stats_name_history_of_a_deleted_printer(async_client: AsyncClient, db_session):
+    base = datetime(2026, 7, 15, 10, 0, 0)
+    db_session.add_all(
+        [
+            PrintLogEntry(
+                printer_id=71,
+                printer_name="Ultron",
+                status="completed",
+                started_at=base,
+                completed_at=base + timedelta(hours=1),
+                duration_seconds=3600,
+            ),
+            PrintLogEntry(
+                printer_id=71,
+                printer_name="Ultron",
+                status="failed",
+                started_at=base + timedelta(hours=2),
+                completed_at=base + timedelta(hours=3),
+                duration_seconds=3600,
+            ),
+        ]
+    )
+    await db_session.commit()
+
+    stats = (await async_client.get("/api/v1/archives/stats")).json()
+
+    assert stats["prints_by_printer"]["71"] == 2
+    assert stats["printer_names"]["71"] == "Ultron"
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_stats_reports_the_last_name_the_printer_ran_under(async_client: AsyncClient, db_session):
+    """A printer renamed before it was deleted is remembered by its last name."""
+    base = datetime(2026, 7, 15, 10, 0, 0)
+    db_session.add_all(
+        [
+            PrintLogEntry(printer_id=71, printer_name="Ultron", status="completed", started_at=base),
+            PrintLogEntry(
+                printer_id=71,
+                printer_name="Ultron Mk II",
+                status="completed",
+                started_at=base + timedelta(days=1),
+            ),
+            # A run logged before names were recorded must not blank the label.
+            PrintLogEntry(printer_id=71, printer_name=None, status="completed", started_at=base + timedelta(days=2)),
+        ]
+    )
+    await db_session.commit()
+
+    stats = (await async_client.get("/api/v1/archives/stats")).json()
+
+    assert stats["printer_names"]["71"] == "Ultron Mk II"
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_stats_names_do_not_override_a_live_printer(async_client: AsyncClient, db_session, printer_factory):
+    """Renaming a printer that still exists shows up straight away.
+
+    The client prefers the live record, so the historical name only has to be
+    present, not authoritative.
+    """
+    printer = await printer_factory(name="Renamed Later")
+    db_session.add(PrintLogEntry(printer_id=printer.id, printer_name="Original Name", status="completed"))
+    await db_session.commit()
+
+    stats = (await async_client.get("/api/v1/archives/stats")).json()
+    printers = (await async_client.get("/api/v1/printers/")).json()
+
+    assert stats["printer_names"][str(printer.id)] == "Original Name"
+    assert [p["name"] for p in printers if p["id"] == printer.id] == ["Renamed Later"]
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_failure_analysis_names_a_deleted_printer(async_client: AsyncClient, db_session, printer_factory):
+    """Failures by printer reads the recorded name once the printer is gone."""
+    live = await printer_factory(name="Still Here")
+    db_session.add_all(
+        [
+            PrintLogEntry(printer_id=71, printer_name="Ultron", status="failed"),
+            PrintLogEntry(printer_id=live.id, printer_name="Older Name", status="failed"),
+        ]
+    )
+    await db_session.commit()
+
+    analysis = (await async_client.get("/api/v1/archives/analysis/failures")).json()
+
+    assert analysis["failures_by_printer"]["Ultron"] == 1
+    assert analysis["failures_by_printer"]["Still Here"] == 1
+    assert "Printer 71" not in analysis["failures_by_printer"]

+ 43 - 1
frontend/src/__tests__/pages/StatsPage.test.tsx

@@ -2,7 +2,7 @@
  * Tests for the StatsPage component.
  */
 
-import { describe, it, expect, beforeEach } from 'vitest';
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import { screen, waitFor } from '@testing-library/react';
 import { render } from '../utils';
 import { StatsPage } from '../../pages/StatsPage';
@@ -621,4 +621,46 @@ describe('StatsPage', () => {
       expect(printTime?.querySelector('svg[aria-label]')).toBeNull();
     });
   });
+
+  describe('deleted printers (#2873)', () => {
+    // The per-printer breakdown only renders at half width, and the dashboard
+    // reads its sizes back from localStorage.
+    const halfWidthSuccessRate = JSON.stringify({
+      order: ['success-rate'],
+      hidden: [],
+      sizes: { 'success-rate': 2 },
+    });
+
+    afterEach(() => {
+      vi.mocked(localStorage.getItem).mockReset();
+    });
+
+    it('labels history from a deleted printer with the name it ran under', async () => {
+      vi.mocked(localStorage.getItem).mockImplementation((key: string) =>
+        key === 'bambusy-dashboard-layout-v2' ? halfWidthSuccessRate : null
+      );
+      server.use(
+        http.get('/api/v1/archives/stats', () =>
+          HttpResponse.json({
+            ...mockStats,
+            // Printer 7 was deleted with its prints kept, so it is missing from
+            // /printers and used to show as "Printer 7".
+            prints_by_printer: { '1': 100, '7': 12 },
+            printer_names: { '1': 'Name At The Time', '7': 'Ultron' },
+          })
+        )
+      );
+
+      render(<StatsPage />);
+
+      await waitFor(() => {
+        expect(screen.getByText('Ultron')).toBeInTheDocument();
+      });
+      expect(screen.queryByText('Printer 7')).not.toBeInTheDocument();
+      // A printer that still exists is named from the live record instead, so a
+      // rename shows up without waiting for the next print.
+      expect(screen.getAllByText('X1 Carbon').length).toBeGreaterThan(0);
+      expect(screen.queryByText('Name At The Time')).not.toBeInTheDocument();
+    });
+  });
 });

+ 3 - 0
frontend/src/api/client.ts

@@ -813,6 +813,9 @@ export interface ArchiveStats {
   total_cost: number;
   prints_by_filament_type: Record<string, number>;
   prints_by_printer: Record<string, number>;
+  // Name each printer id was last recorded under in the print log, so history
+  // belonging to a deleted printer keeps its label.
+  printer_names?: Record<string, string>;
   average_time_accuracy: number | null;
   time_accuracy_by_printer: Record<string, number> | null;
   total_energy_kwh: number;

+ 12 - 1
frontend/src/pages/StatsPage.tsx

@@ -1103,7 +1103,18 @@ export function StatsPage() {
   const isRefetching = (isStatsFetching || isArchivesFetching) && !isLoading;
 
   const currency = getCurrencySymbol(settings?.currency || 'USD');
-  const printerMap = new Map(printers?.map((p) => [String(p.id), p.name]) || []);
+  // History outlives the printer that made it: a deleted printer is gone from
+  // `printers`, so its rows fell back to "Printer 1" (#2873). The stats response
+  // carries the name each id was last recorded under; a printer that still
+  // exists overrides it, so a rename shows up straight away.
+  const printerMap = useMemo(
+    () =>
+      new Map<string, string>([
+        ...Object.entries(stats?.printer_names || {}),
+        ...(printers?.map((p) => [String(p.id), p.name] as [string, string]) || []),
+      ]),
+    [stats?.printer_names, printers],
+  );
   const printDates = useMemo(() => archives?.map((a) => a.created_at) || [], [archives]);
 
   if (isLoading) {

File diff suppressed because it is too large
+ 0 - 0
static/assets/index-DwcyDXDd.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-CVKc5Khd.js"></script>
+    <script type="module" crossorigin src="/assets/index-DwcyDXDd.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-kSJGQrMr.css">
   </head>
   <body>

Some files were not shown because too many files changed in this diff