mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 11:12:35 +02:00
fix(notifications): missing-spool-assignment check now unions both assignment tables (#1473)
notify_missing_spool_assignments_on_print_start queried only the legacy SpoolAssignment table. In Spoolman mode that table is empty -- bindings live in spoolman_slot_assignments -- so assigned_global_trays came back empty and every used tray was flagged missing, firing a false-positive notification on every print. Union SpoolAssignment + SpoolmanSlotAssignment rows for the printer before computing the missing set. Both tables expose printer_id / ams_id / tray_id identically, so _global_tray_from_assignment is unchanged. Union-only, so legacy-mode behavior cannot regress.
This commit is contained in:
@@ -15,6 +15,8 @@ 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
|
||||
- **Missing-spool-assignment notification no longer false-fires on every Spoolman-mode print (#1473, reported and root-caused by @ojimpo)** — Reporter on Spoolman mode (AMS 2 Pro, all four trays bound to Spoolman spools via the Assign-Spool UI) got a `print_missing_spool_assignment` notification on every print start — 13 false positives in 7 days — each flagging trays that were correctly bound. He traced it precisely: `backend/app/services/spool_assignment_notifications.py` queried only the legacy `SpoolAssignment` table, never `SpoolmanSlotAssignment`. In Spoolman mode the legacy table is empty (bindings live in `spoolman_slot_assignments`, the source-of-truth since #1119), so `assigned_global_trays` came back empty and every used tray was reported missing. Same class of miss as #1459 (the weight tracker also skipped `SpoolmanSlotAssignment`) and a [[feedback_inventory_modes_parity]] violation — a check present in both modes was wired only for legacy. **Fix**: the assigned-tray set is now the **union** of both tables — `SpoolAssignment` and `SpoolmanSlotAssignment` rows for the printer. Both expose `printer_id` / `ams_id` / `tray_id` in identical shape (verified against `models/spoolman_slot_assignment.py`, whose `ams_id` range 0-7 / 128-191 / 255 is fully covered by the existing `_global_tray_from_assignment()`), so the helper works on either unchanged. The union is strictly safe: it can only *add* assignments, so it never regresses legacy-mode behavior and never reports a genuinely-unassigned tray as covered. Scope note: this does not add RFID-`extra.tag` resolution (a tray bound purely via the loaded spool's RFID tag with no slot-assignment row) — that needs the Spoolman client and is a deeper change; the reported false positive is entirely covered by the union since the Assign-Spool UI writes `SpoolmanSlotAssignment`. **Tests**: 3 new in `test_spool_assignment_notifications.py` (the reporter's suggested cases) — Spoolman-only binding suppresses the notification; Spoolman partial coverage flags only the uncovered tray; mixed-mode (A1 legacy + A2 Spoolman) union covers all used trays. The test fake now routes `execute()` by target table so either mode can be exercised; the existing legacy-mode test still passes unchanged. 4 notification tests green; backend ruff clean.
|
||||
|
||||
- **Local Profiles: the search bar no longer disappears when a query matches nothing (#1470, reported by @pwostran)** — Typing a query in Settings → Local Profiles that matched no preset made the search bar itself vanish, leaving the user unable to clear or edit the query without a full page refresh. Root cause in `frontend/src/components/LocalProfilesView.tsx`: the search bar was gated on `{totalCount > 0 && …}`, and `totalCount` is the sum of the *post-filter* `filaments` / `printers` / `processes` lengths — so the moment the query filtered every column to empty, `totalCount` hit 0 and the search bar unmounted along with the columns. The `totalCount === 0` "No local presets yet" empty state then took over, which also misleadingly implied nothing was imported. **Fix**: added `hasAnyPresets`, computed from the *pre-filter* preset counts (`presets?.filament/printer/process` lengths), and gated the search bar on that instead — it stays mounted as long as any preset exists, regardless of the query. The empty state is now split: `!hasAnyPresets` shows the genuine "No local presets yet" + import hint, while `hasAnyPresets && totalCount === 0` shows a new "No presets match your search" message (with a search icon) so the two cases are no longer conflated. New `noSearchResults` i18n key added with real translations in all 8 locales (en/de/fr/it/ja/pt-BR/zh-CN/zh-TW). **Tests**: 1 new in `LocalProfilesView.test.tsx` — types a non-matching query and asserts the search bar is still in the DOM, retains the typed value, and the no-matches message renders. 10 LocalProfilesView tests green; i18n parity 4859 keys × 8 locales; frontend build clean.
|
||||
|
||||
- **Failure Detection: the Status panel's Low / High thresholds now reflect the selected sensitivity (#1469, reported by @JohnMacOB)** — Reporter changed the Sensitivity dropdown (Low / Medium / High) in Settings → Failure Detection and the "Low / High thresholds" readout in the Status panel never moved off `0.38 / 0.78`, so the setting looked dead. **Detection itself was always correct** — the classifier at `backend/app/services/obico_detection.py:280` uses `classify(score, settings["sensitivity"])` with the real value, so warnings/failures triggered at the right confidence for the chosen level. The bug was display-only: `ObicoDetectionService.get_status()` computed the displayed thresholds with a hardcoded `thresholds("medium")` (`obico_detection.py:324`), ignoring the configured sensitivity. `thresholds()` is `BASE × SENSITIVITY_MULT` — low ×1.25 → `0.48 / 0.98`, medium ×1.0 → `0.38 / 0.78`, high ×0.75 → `0.29 / 0.59` — so the panel always showed the medium row whatever the user picked, making a working setting look broken. **Fix**: `get_status()` takes an optional `sensitivity` parameter (default `"medium"`, so `thresholds()`'s own unknown-value fallback still applies) and the `/obico/status` route — which already loads settings fresh and has `settings["sensitivity"]` in hand — passes it through. The readout now updates the instant the dropdown change is saved (the frontend already invalidates the `obico-status` query on save), with no wait for the next poll cycle. **Tests**: 1 new in `test_obico_detection.py::TestGetStatus` — `test_thresholds_reflect_configured_sensitivity` asserts low > medium > high for both threshold bounds and that the default / unknown sensitivity falls back to medium. 47 obico unit tests + 5 obico API integration tests green; backend ruff clean.
|
||||
|
||||
@@ -4,6 +4,7 @@ from backend.app.core.database import async_session
|
||||
from backend.app.core.websocket import ws_manager
|
||||
from backend.app.models.printer import Printer
|
||||
from backend.app.models.spool_assignment import SpoolAssignment
|
||||
from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment
|
||||
from backend.app.services.bambu_mqtt import PrinterState
|
||||
from backend.app.services.notification_service import notification_service
|
||||
from backend.app.services.printer_manager import printer_manager
|
||||
@@ -127,12 +128,23 @@ async def notify_missing_spool_assignments_on_print_start(
|
||||
printer = await db.get(Printer, printer_id)
|
||||
printer_name = printer.name if printer else f"Printer {printer_id}"
|
||||
|
||||
assignments_result = await db.execute(
|
||||
SpoolAssignment.__table__.select().where(SpoolAssignment.printer_id == printer_id)
|
||||
)
|
||||
assignments = assignments_result.fetchall()
|
||||
# A tray is "assigned" if it has a row in EITHER table: the legacy
|
||||
# spool_assignment table (internal-inventory mode) or
|
||||
# spoolman_slot_assignments (Spoolman mode — the binding
|
||||
# source-of-truth since #1119). Querying only the legacy table
|
||||
# flagged every used tray as missing on every Spoolman-mode print
|
||||
# (#1473). Both tables expose printer_id / ams_id / tray_id in the
|
||||
# same shape, so _global_tray_from_assignment works on either.
|
||||
legacy_rows = (
|
||||
await db.execute(SpoolAssignment.__table__.select().where(SpoolAssignment.printer_id == printer_id))
|
||||
).fetchall()
|
||||
spoolman_rows = (
|
||||
await db.execute(
|
||||
SpoolmanSlotAssignment.__table__.select().where(SpoolmanSlotAssignment.printer_id == printer_id)
|
||||
)
|
||||
).fetchall()
|
||||
assigned_global_trays = {
|
||||
_global_tray_from_assignment(assignment.ams_id, assignment.tray_id) for assignment in assignments
|
||||
_global_tray_from_assignment(row.ams_id, row.tray_id) for row in (*legacy_rows, *spoolman_rows)
|
||||
}
|
||||
|
||||
missing_global = sorted(used_global_trays - assigned_global_trays)
|
||||
|
||||
@@ -18,9 +18,18 @@ class _FakeAssignmentsResult:
|
||||
|
||||
|
||||
class _FakeSession:
|
||||
def __init__(self, printer_name: str, assignments: list[SimpleNamespace]):
|
||||
"""Fake DB session that returns legacy vs. Spoolman assignment rows based
|
||||
on which table the SELECT targets, so tests can exercise either mode."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
printer_name: str,
|
||||
legacy: list[SimpleNamespace] | None = None,
|
||||
spoolman: list[SimpleNamespace] | None = None,
|
||||
):
|
||||
self._printer = SimpleNamespace(name=printer_name)
|
||||
self._assignments = assignments
|
||||
self._legacy = legacy or []
|
||||
self._spoolman = spoolman or []
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
@@ -32,7 +41,10 @@ class _FakeSession:
|
||||
return self._printer
|
||||
|
||||
async def execute(self, statement):
|
||||
return _FakeAssignmentsResult(self._assignments)
|
||||
table = statement.get_final_froms()[0].name
|
||||
if table == "spoolman_slot_assignments":
|
||||
return _FakeAssignmentsResult(self._spoolman)
|
||||
return _FakeAssignmentsResult(self._legacy)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -75,3 +87,85 @@ async def test_missing_assignment_broadcasts_websocket_event_and_push_notificati
|
||||
assert notify_kwargs["printer_id"] == 1
|
||||
assert notify_kwargs["printer_name"] == "Printer A"
|
||||
assert notify_kwargs["missing_slots"] == [{"slot": "A2", "profile": "Unknown", "color": "Unknown"}]
|
||||
|
||||
|
||||
def _patches(session):
|
||||
"""Common patch set: the fake session + stubbed printer state / emitters."""
|
||||
return (
|
||||
patch(
|
||||
"backend.app.services.spool_assignment_notifications.async_session",
|
||||
return_value=session,
|
||||
),
|
||||
patch("backend.app.services.spool_assignment_notifications.printer_manager.get_status", return_value=None),
|
||||
patch(
|
||||
"backend.app.services.spool_assignment_notifications.ws_manager.send_missing_spool_assignment",
|
||||
new_callable=AsyncMock,
|
||||
),
|
||||
patch(
|
||||
"backend.app.services.spool_assignment_notifications.notification_service.on_print_missing_spool_assignment",
|
||||
new_callable=AsyncMock,
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_spoolman_only_assignment_suppresses_notification():
|
||||
"""#1473 — trays bound only via spoolman_slot_assignments must NOT be
|
||||
flagged missing (the legacy spool_assignment table is empty in Spoolman
|
||||
mode, so checking it alone fired a false positive on every print)."""
|
||||
logger = logging.getLogger(__name__)
|
||||
data = {"ams_mapping": [0, 1], "raw_data": {}} # print uses A1 + A2
|
||||
|
||||
# Both used trays bound via Spoolman; legacy table empty.
|
||||
session = _FakeSession(
|
||||
"Printer A",
|
||||
legacy=[],
|
||||
spoolman=[SimpleNamespace(ams_id=0, tray_id=0), SimpleNamespace(ams_id=0, tray_id=1)],
|
||||
)
|
||||
p_session, p_status, p_ws, p_notify = _patches(session)
|
||||
with p_session, p_status, p_ws as mock_ws, p_notify as mock_notify:
|
||||
await notify_missing_spool_assignments_on_print_start(1, data, logger)
|
||||
|
||||
mock_ws.assert_not_awaited()
|
||||
mock_notify.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_spoolman_partial_coverage_flags_only_uncovered_tray():
|
||||
"""A Spoolman assignment for A1 only, with a print using A1 + A2, flags
|
||||
A2 alone."""
|
||||
logger = logging.getLogger(__name__)
|
||||
data = {"ams_mapping": [0, 1], "raw_data": {}}
|
||||
|
||||
session = _FakeSession(
|
||||
"Printer A",
|
||||
legacy=[],
|
||||
spoolman=[SimpleNamespace(ams_id=0, tray_id=0)], # A1 only
|
||||
)
|
||||
p_session, p_status, p_ws, p_notify = _patches(session)
|
||||
with p_session, p_status, p_ws as mock_ws, p_notify as mock_notify:
|
||||
await notify_missing_spool_assignments_on_print_start(1, data, logger)
|
||||
|
||||
mock_ws.assert_awaited_once()
|
||||
assert mock_ws.await_args.kwargs["missing_slots"] == [{"slot": "A2", "profile": "Unknown", "color": "Unknown"}]
|
||||
mock_notify.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mixed_mode_union_covers_all_used_trays():
|
||||
"""A1 bound in the legacy table, A2 bound in spoolman_slot_assignments —
|
||||
the union covers both used trays, so no notification fires."""
|
||||
logger = logging.getLogger(__name__)
|
||||
data = {"ams_mapping": [0, 1], "raw_data": {}}
|
||||
|
||||
session = _FakeSession(
|
||||
"Printer A",
|
||||
legacy=[SimpleNamespace(ams_id=0, tray_id=0)], # A1
|
||||
spoolman=[SimpleNamespace(ams_id=0, tray_id=1)], # A2
|
||||
)
|
||||
p_session, p_status, p_ws, p_notify = _patches(session)
|
||||
with p_session, p_status, p_ws as mock_ws, p_notify as mock_notify:
|
||||
await notify_missing_spool_assignments_on_print_start(1, data, logger)
|
||||
|
||||
mock_ws.assert_not_awaited()
|
||||
mock_notify.assert_not_awaited()
|
||||
|
||||
Reference in New Issue
Block a user