mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 03:54:17 +02:00
fix(stats): cancelled bucket icon now uses semantic warning token (orange) (#1390 follow-up)
Reporter flagged that the new Cancelled row's Ban icon rendered colourless while Successful and Failed used green/red icons in the same widget. Switch the Cancelled icon to text-status-warning (amber-500) so all three rows now use semantic status tokens consistently, and the colour matches the orange Archives + notification badges already use for cancelled status.
This commit is contained in:
+1
-1
@@ -28,7 +28,7 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Trivy DS-0026 (`Dockerfile.test` missing HEALTHCHECK): silenced via `HEALTHCHECK NONE`** — The test image runs `pytest` and exits; there is no long-running service to probe, so any HEALTHCHECK we added would be cargo-cult noise. `HEALTHCHECK NONE` is the documented Docker directive to explicitly opt out of any inherited healthcheck and is the way Trivy expects projects to signal "this image is not a service." Closes code-scanning alert #813.
|
||||
|
||||
### Fixed
|
||||
- **Quick Stats: user-cancelled prints now have their own bucket and no longer drag down the Success Rate gauge (#1390 follow-up, reported by @IndividualGhost1905)** — Reporter saw `Total prints: 20 / Success: 18 / Failed: 1` and asked where the 20th print went; the breakdown only showed Successful + Failed, so a cancelled run silently inflated the total without appearing anywhere. The earlier #1390 round had committed a test that *locked in* the bug — `it('uses total_prints as denominator so cancelled/stopped events count')` asserted the gauge should divide by `total_prints`, which lumped user/queue-cancelled jobs in with quality outcomes and conflated user intent with printer performance. **Cause**: `PrintLogEntry.status` has six values in production (`completed`, `failed`, `aborted`, `stopped`, `cancelled`, `skipped`) but the Quick Stats endpoint in `api/routes/archives.py` only counted two — `completed` → Successful, `status == "failed"` → Failed — and used a raw `count(*)` for Total Prints, so the other four statuses ended up in Total without surfacing in any breakdown row. `aborted` was particularly silent: classified as a failure elsewhere in the codebase (`failure_analysis.py`, `main.py:430,1729`) but not counted toward `failed_prints` in stats. **Fix**: three-bucket classification across the whole stats surface, matching how the rest of the codebase already groups these statuses. Quick Stats now returns `successful_prints` (completed), `failed_prints` (failed + aborted — printer-detected quality failures), and a new `cancelled_prints` (stopped + cancelled + skipped — user/queue interruptions). The SuccessRateWidget gauge divides by `successful + failed` only, so cancelling a roll because you changed your mind doesn't ding the printer's success rate — a Cancelled row in the breakdown surfaces the count so it doesn't silently vanish from Total Prints. The Failure Analysis service applies the same denominator change (`failure_rate = failed / (successful + failed)`) to both the headline rate and the per-week trend, so a week with no failures but several cancellations no longer reads as a misleading 0/N. **Schema change is additive-safe**: `ArchiveStats.cancelled_prints` defaults to `0` so any historical fixture validating against the model still parses; the frontend type also defaults the display to `0` when the field is missing. **i18n**: new `stats.cancelled` key with real translations across all 9 locales (de/es/fr/it/ja/pt-BR/zh-CN/zh-TW) per [[feedback_translate_dont_fallback]]; parity script clean at 4994 leaves per locale. **Tests**: existing `it('uses total_prints as denominator …')` test inverted to assert the new behaviour (40 completed / 20 failed / 35 cancelled → gauge shows 67%, Cancelled row reads 35), `cancelled_prints: 0` added to the shared mock so the unchanged-display assertion (140/150 → 93%) still holds since `140 / (140 + 10) = 93.33%` rounds identically. 33 StatsPage tests + 6 backend stats/failure tests green; frontend build + backend ruff clean.
|
||||
- **Quick Stats: user-cancelled prints now have their own bucket and no longer drag down the Success Rate gauge (#1390 follow-up, reported by @IndividualGhost1905)** — Reporter saw `Total prints: 20 / Success: 18 / Failed: 1` and asked where the 20th print went; the breakdown only showed Successful + Failed, so a cancelled run silently inflated the total without appearing anywhere. The earlier #1390 round had committed a test that *locked in* the bug — `it('uses total_prints as denominator so cancelled/stopped events count')` asserted the gauge should divide by `total_prints`, which lumped user/queue-cancelled jobs in with quality outcomes and conflated user intent with printer performance. **Cause**: `PrintLogEntry.status` has six values in production (`completed`, `failed`, `aborted`, `stopped`, `cancelled`, `skipped`) but the Quick Stats endpoint in `api/routes/archives.py` only counted two — `completed` → Successful, `status == "failed"` → Failed — and used a raw `count(*)` for Total Prints, so the other four statuses ended up in Total without surfacing in any breakdown row. `aborted` was particularly silent: classified as a failure elsewhere in the codebase (`failure_analysis.py`, `main.py:430,1729`) but not counted toward `failed_prints` in stats. **Fix**: three-bucket classification across the whole stats surface, matching how the rest of the codebase already groups these statuses. Quick Stats now returns `successful_prints` (completed), `failed_prints` (failed + aborted — printer-detected quality failures), and a new `cancelled_prints` (stopped + cancelled + skipped — user/queue interruptions). The SuccessRateWidget gauge divides by `successful + failed` only, so cancelling a roll because you changed your mind doesn't ding the printer's success rate — a Cancelled row in the breakdown surfaces the count so it doesn't silently vanish from Total Prints. The Failure Analysis service applies the same denominator change (`failure_rate = failed / (successful + failed)`) to both the headline rate and the per-week trend, so a week with no failures but several cancellations no longer reads as a misleading 0/N. **Schema change is additive-safe**: `ArchiveStats.cancelled_prints` defaults to `0` so any historical fixture validating against the model still parses; the frontend type also defaults the display to `0` when the field is missing. **i18n**: new `stats.cancelled` key with real translations across all 9 locales (de/es/fr/it/ja/pt-BR/zh-CN/zh-TW) per [[feedback_translate_dont_fallback]]; parity script clean at 4994 leaves per locale. **Tests**: existing `it('uses total_prints as denominator …')` test inverted to assert the new behaviour (40 completed / 20 failed / 35 cancelled → gauge shows 67%, Cancelled row reads 35), `cancelled_prints: 0` added to the shared mock so the unchanged-display assertion (140/150 → 93%) still holds since `140 / (140 + 10) = 93.33%` rounds identically. 33 StatsPage tests + 6 backend stats/failure tests green; frontend build + backend ruff clean. **Follow-up (cosmetic):** the new Cancelled row's Ban icon rendered in `text-bambu-gray` while the Successful and Failed icons used semantic `text-status-ok` / `text-status-error` tokens — reporter (@IndividualGhost1905) noted the asymmetry and asked for an orange to match what Archives + notification badges use for cancelled. Switched the Cancelled row to `text-status-warning` (amber-500, same token family as the other two rows), so all three icons are now semantic-token-driven and the new row matches the colour the user already associates with cancelled status elsewhere in the UI.
|
||||
- **Support bundle + bug-report submission now include the live diagnostic snapshot** — Three diagnostics (Connection Diagnostic per printer, Virtual Printer Setup Diagnostic per enabled VP, Log Health Scanner) have shipped on the System page and inline in the bug-report bubble since 6bc6a1d6 / e222a0ef / ed31b8f4, but the results were only ever shown to the *user* — never persisted into the downloadable support ZIP or the submitted GitHub issue. A report saying "looks broken in Bambuddy" arrived with no actionable signal beyond raw logs. **Fix**: new `services/diagnostic_snapshot.collect_diagnostic_snapshot` runs all three concurrently with an outer per-probe 15 s wall-clock cap (so a hung interface adds at most ~15 s to bundle generation regardless of fleet size — `asyncio.gather`, total ≈ max(per-cap) not sum). Fail-soft per probe: a crash inside one printer's check emits `{"printer_id": N, "error": "..."}` for that entry rather than nuking the whole snapshot — partial result beats a 500. Wired into `_collect_support_info()` so both flows (`POST /support/bundle` and `POST /bug-report/submit` via `support_info=...`) pick up the new `diagnostics` top-level key without their own changes. **Private-data sanitization** — the diagnostic schemas embed raw IPv4 in three places (`PrinterDiagnosticResult.ip_address`, network-mode check's `params.{printer_ip, host_ip}`, VP diagnostic's `params.bind_ip`), and the snapshot adds printer names. None of those should leak. The snapshot now runs a recursive sanitizer on the full result tree before returning: known DB-listed values (printer name, IP, serial, access code) get the same `[PRINTER]/[IP]/[SERIAL]/[ACCESS_CODE]` labels the log sanitizer already applies (via the shared `collect_sensitive_strings`), and an IPv4-regex fallback catches IPs the DB doesn't know about — most importantly the Bambuddy host IP returned by `_get_host_ip()` and any VP `bind_ip` the user picked at setup. Live-DB smoke test confirms zero raw IPv4 instances in the serialized snapshot output. **Progress indicators**: the bubble's "submitting" view and the System page's Download button now render a static four-line checklist showing what's running (printer connectivity → VP setup → log scan → submit/build ZIP) — communicates the longer wait honestly without faking server-side phase progress we can't actually track. **Tests**: 6 new in `test_diagnostic_snapshot.py` — empty-input shape stable, per-printer / per-VP result coverage, fail-soft on a single-probe crash, `timed_out` marker when a probe exceeds the per-probe cap (test patches the cap to 0.05 s), end-to-end IP sanitization across all five field shapes (top-level `ip_address`, `printer_ip`, `host_ip`, `bind_ip`, plus IPs embedded in log-health sample lines) with a final regex sweep over the JSON-serialized result asserting zero raw IPv4 escapes, concurrent execution proof (4 × 0.2 s probes complete in < 0.5 s, would be 0.8 s sequential). Existing 27 BugReportBubble + SystemInfoPage frontend tests still pass; 9-locale i18n parity check clean (4993 leaves per locale, 9 new keys added with real translations everywhere — no English fallback). Backend ruff clean.
|
||||
- **"Prefer Lowest Remaining Filament" now uses Bambuddy's inventory weight, not just the printer's RFID counter (#1508, reported by @kleinwareio)** — Reporter has an inventory spool cloned to slot 1 and the original (much further used) in slot 4 of the same P1S AMS, with the preference enabled, and the dispatch consistently picked slot 1 (the fresh clone) instead of slot 4 (the original they wanted to finish first). Root cause is the `prefer_lowest` sort in `_match_filaments_to_slots` (`print_scheduler.py`): the sort key reads `f.get("remain", -1)` straight out of `_build_loaded_filaments`, which sources it from MQTT AMS `tray.remain` — the printer firmware's own RFID-decremented value. Two problems with that signal: (a) it's only populated for Bambu RFID spools, so every non-RFID / 3rd-party / user-loaded tray reports `-1` and gets clamped to a sentinel — multiple non-RFID spools then tie in the sort and Python's stable sort collapses to AMS-slot insertion order, so slot 1 always wins; (b) even when set, it's the *printer's* counter, not Bambuddy's `label_weight - weight_used` (internal mode) or Spoolman's `remaining_weight` (Spoolman mode) — the two diverge any time the user re-spools, swaps cardboard, or runs a print outside Bambuddy. The reporter is on internal-inventory mode with non-RFID spools — both failure modes apply, hence slot 1 every time. **Fix**: when a slot is bound to a Bambuddy / Spoolman spool, that inventory record's remaining weight becomes the sort signal. New async helper `_build_inventory_remain_overrides(db, printer_id, loaded)` returns `{global_tray_id: remaining_grams}` for slots with an assignment — internal mode joins `SpoolAssignment` → `Spool` once per dispatch, Spoolman mode joins `SpoolmanSlotAssignment` then fetches each spool through the existing `_spoolman_remaining_grams` (shared with `filament_deficit.py`, parity rule per [[feedback_inventory_modes_parity]]). The new `_prefer_lowest_sort_key` consumes that map alongside the legacy MQTT field with a **two-tier** comparison: inventory-tracked spools always sort BEFORE MQTT-only spools, then ascending by remaining within each tier, then ascending by `ams_id * 4 + tray_id` as the deterministic slot tie-breaker. The tier flag dominates so we never compare grams (inventory) against percent (MQTT) — no unit-conversion contortions. MQTT-only behaviour is preserved exactly: `remain = -1` still maps to the 101 sentinel and slot order still decides on ties, so users who haven't bound any spools see no change. External / VT tray slots are skipped (tracked separately from AMS bindings). Lookup runs only when `prefer_lowest_filament` is enabled — no extra DB hit for users who don't use the preference. **Tests**: 6 new in `TestPreferLowestInventoryOverride` in `test_scheduler_ams_mapping.py` (inventory override beats MQTT remain — the literal reporter scenario with 950 g clone vs 50 g original; zero-grams still sorts first within its tier; inventory tier beats MQTT tier regardless of value; tied inventory grams break to lower slot; no-override falls through to MQTT — regression guard for un-tracked spools; legacy `remain = -1` still sentinel-sorts last when override map is None) + 7 new in `test_scheduler_inventory_remain.py` covering `_build_inventory_remain_overrides` directly (internal mode returns label_weight − weight_used per bound slot; external slots skipped; empty loaded short-circuits; over-consumed spool clamps to 0 g; unbound slots absent from map; Spoolman mode uses `_spoolman_remaining_grams` for parity; Spoolman unreachability silently omits that slot). 102 scheduler + inventory tests green; backend ruff clean.
|
||||
- **X1/H2/P2 live camera no longer fails with `Address already in use` on transitional ffmpeg builds (#1504, reported by @rage03usa, confirmed by @PawseHaxor)** — On a native Ubuntu install with the Jammy-era system ffmpeg, the RTSP live-view path retried indefinitely with `Unable to open RTSP for listening … Address already in use`. Snapshots, the camera diagnostic, and OrcaSlicer all kept working — only live view was broken. Cause: the ffmpeg argv built in `backend/app/api/routes/camera.py` (added in 530a7a46 as part of an RTSP-stability bundle) passed `-timeout 30000000`. That ffmpeg version *deprecated* the original `-timeout` (socket I/O microseconds) and repurposed the name to mean the *RTSP listen-mode incoming-connection timeout* — any non-zero value **implies `-listen`**. ffmpeg then flipped into RTSP server mode and tried to bind the same localhost port Bambuddy's TLS proxy was already listening on, hence EADDRINUSE on every retry (the odd-port pattern @PawseHaxor noticed is coincidence — the ephemeral allocator just picked odd values that run). The reporter's own workaround (drop the option) works but silently loses the socket-level read timeout, so a hung TLS handshake would block past the OS TCP timeout instead of failing fast into the existing reconnect loop. **Why this can't be a one-line literal swap**: ffmpeg has shipped *three* arrangements of this option over time and Bambuddy supports the full range. Pre-deprecation builds: `-timeout` is the socket I/O timeout. Transitional builds (~late-4.x, what the reporter is on): `-timeout` is the broken listen-mode option, `-stimeout` is the replacement. **Modern ffmpeg (5.x / 6.x / 7.x — current Debian 13, Ubuntu 24.04, current Homebrew)**: `-stimeout` was removed entirely and `-timeout` is back to socket I/O. So both literals regress one half of the install base. **Fix**: a new `rtsp_socket_timeout_flag()` helper in `backend/app/services/camera.py` probes `ffmpeg -h demuxer=rtsp` once at first use and picks `-stimeout` when ffmpeg advertises it (transitional window) or `-timeout` otherwise (modern + very old). The result is cached for the process lifetime — ffmpeg won't swap mid-run. The function returns the option name without a leading dash so callers prepend it themselves (no empty-flag formatting bug). Wired into both RTSP ffmpeg call sites — `routes/camera.py` (printer camera) and `services/external_camera.py` (external RTSP) — in lockstep, same TLS-proxy + ffmpeg pattern, same regression. The reporter had tried `-listen_timeout` (doesn't help — we *don't* want listen mode) and `-rw_timeout` (AVIO-level, RTSP demuxer doesn't honour it on its control socket), but no manual swap could be correct for both transitional and modern installs simultaneously. **Tests**: 8 in `test_ffmpeg_rtsp_timeout_flag.py` — 6 unit tests for the probe (picks `-stimeout` when advertised, falls back to `-timeout` on modern, defaults to `-timeout` when ffmpeg missing or probe raises, caches across calls, substring-match guard against false-positives on `-listen_timeout`), 2 parametrised regression guards against either RTSP ffmpeg argv re-hard-coding a literal flag instead of consuming the probe. 37 (probe + existing external-camera) tests green; backend ruff clean.
|
||||
|
||||
@@ -253,7 +253,7 @@ function SuccessRateWidget({
|
||||
<span className="text-sm text-white font-medium">{stats?.failed_prints || 0}</span>
|
||||
</div>
|
||||
<div className="flex items-center gap-2">
|
||||
<Ban className="w-4 h-4 text-bambu-gray flex-shrink-0" />
|
||||
<Ban className="w-4 h-4 text-status-warning flex-shrink-0" />
|
||||
<span className="text-sm text-bambu-gray">{t('stats.cancelled')}</span>
|
||||
<span className="text-sm text-white font-medium">{stats?.cancelled_prints || 0}</span>
|
||||
</div>
|
||||
|
||||
File diff suppressed because one or more lines are too long
+1
-1
@@ -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-BW850Rth.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-RpiIrueH.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-y4woBlMv.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user