fix(library-tags): declare response_model=None on the 204 DELETE route

Under `from __future__ import annotations` the `-> None` return annotation
reaches FastAPI as the string "None", which resolves to NoneType -- truthy,
so APIRoute asserts a 204 may carry no response body and the app fails to
import. fastapi >= 0.116 guards against this; the 0.109-0.115 releases
requirements.txt still allows do not.
This commit is contained in:
maziggy
2026-07-09 16:29:13 +02:00
parent 179b46580f
commit e1e2c12d25
2 changed files with 8 additions and 1 deletions
+2
View File
@@ -37,6 +37,8 @@ All notable changes to Bambuddy will be documented in this file.
- **Sort File Manager folder tree by recent activity (#1770, requested by @Kingbuzz0)** — Until now the folder tree was always sorted alphabetically by name, both backend (`order_by(LibraryFolder.name)`) and frontend. The reporter — a user with a lot of nested cad / slicer directories — wanted "find folders that just got a new 3MF" without scrolling the whole alphabet. **What changed.** The folder sidebar header gains a small dropdown (**By name** / **By recent activity**) plus an asc / desc arrow button, sitting alongside the existing Collapse + Wrap toggles. Choice persists per-browser via `localStorage` (`library-folder-sort-field`, `library-folder-sort-direction`) so the preference survives reloads. **Activity semantics.** `latest_activity_at` per folder = `MAX(folder.updated_at, MAX(immediate-child file.updated_at))`. The DB had the data — `LibraryFile.updated_at` is `onupdate=func.now()` and `LibraryFolder.updated_at` the same — but `LibraryFolder.updated_at` alone only bumps on rename / move, not on file-add inside the folder, which is exactly the wrong signal for "did I just drop a new model in here." The aggregate fixes that. Recursion across subfolders is intentionally **NOT** computed — a deeply nested new 3MF bubbles its immediate parent, not every ancestor up to the root. This keeps the route a single `GROUP BY` rather than a recursive CTE, matching the existing file_counts subquery shape sibling at `library.py:746`. A future Tier 3 follow-up could add the recursive-CTE variant if anyone reports deeply-nested updates not bubbling far enough. **Backend.** New `latest_activity_at: datetime | None` field on `FolderResponse` and `FolderTreeItem` schemas. The `/folders` tree route picks up a sibling `func.max(LibraryFile.updated_at)` group-by alongside the existing file-count subquery; resolves the field per row. The `/folders/by-project/{id}` and `/folders/by-archive/{id}` routes collapse their per-row file-count subquery to fetch `count + max` in one trip (one extra column, zero extra round-trips). All 5 single-folder constructors (POST `/folders`, GET `/folders/{id}`, PUT `/folders/{id}`, POST `/folders/external`, the create flows) populate the field with `max(folder.updated_at, latest_file)` or fall back to `folder.updated_at` when there are no files, so the API surface is consistent across every route that returns a folder. **External folders.** `LibraryFile` rows are created for scanned external files too (`library.py:526`), so the MAX aggregate works on them — but the timestamp reflects when Bambuddy last *scanned / re-indexed* the file, not the filesystem mtime. For a NAS that gets new files added outside Bambuddy, the activity-sort lags until the next scan. Documented in the file-manager wiki page rather than papered over with `os.stat()` on every list call, which would stall the route on slow mounts. **Frontend.** A new recursive `sortedFolders` `useMemo` applies the comparator uniformly to top-level + every nested `children` level so sort order is consistent at every depth. Comparator falls back to name when activity timestamps tie or are both null, so an empty folder never elbows a recently-used one to a random place — empties go to the end of the activity bucket regardless of direction. Both the desktop sidebar render and the mobile selector dropdown consume `sortedFolders` so the order is identical across breakpoints. The single-folder `findFolder()` traversal and `selectedFolder` memo still operate on the unsorted `folders` because they index by ID — sort-order-independent. **Recursion safety.** The sort creates fresh object refs at every level on every memo invocation; the `FolderTreeItem` keys stay ID-based (`${folder.id}-${collapseFoldersByDefault ? 'c' : 'e'}`) so React reconciliation by ID preserves folder expansion state across sort flips. **i18n.** 3 new keys in `fileManager.*` (`folderSort`, `folderSortByName`, `folderSortByActivity`) translated in all 11 locales (de / en / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW), no English fallback. Parity 5238 leaves per locale. **Tests.** 2 new backend integration cases in `test_library_api.py` (file-in-folder bubbles `latest_activity_at` to the file's timestamp, empty folder falls back to `folder.updated_at`). All 152 library + folder + trash + slice integration tests still pass; 51/51 FileManagerPage frontend tests still pass; 26/26 QueuePage tests still pass; `npm run build` clean; `ruff` clean; i18n parity green.
### Fixed
- **Backend failed to start on fastapi < 0.116: `AssertionError: Status code 204 must not have a response body`** — `uvicorn backend.app.main:app` aborted at import time while registering `DELETE /api/v1/library/tags/{tag_id}`. The route is declared `status_code=204` with a `-> None` return annotation, and `library_tags.py` uses `from __future__ import annotations` — so the annotation reaches FastAPI as the *string* `"None"`, which `get_typed_annotation()` resolves via `evaluate_forwardref()` to `NoneType`. `NoneType` is a class and therefore truthy, so `APIRoute.__init__` took the `if self.response_model:` branch and asserted that a 204 may carry no response body. fastapi **0.116** added an `if annotation is type(None): return None` guard that makes this benign, which is why CI and the Docker image (both resolve the top of the `>=0.109.0,<0.136.0` range) never saw it — only installs pinned to an older release inside that supported range, such as a venv created before the tag catalog landed in #1268, hit the crash. **Fix.** The route declares `response_model=None` explicitly, which short-circuits the annotation inference on every fastapi version in the supported range. The sibling 204 route (`DELETE /slicer/pipelines/{pipeline_id}`) is unaffected — its module has no `from __future__ import annotations` and no return annotation. **Scope.** Backend-only, one decorator. No behaviour change on fastapi >= 0.116, no DB migration, no new permission, no i18n change. Existing installs can equivalently unblock themselves with `pip install -U -r requirements.txt`.
- **Dependency floors permitted resolutions the code can't run on: `sqlalchemy>=2.0.38`, exact ruff pin** — Two more instances of the same class of defect as the 204 crash above: `requirements.txt` declared floors low enough that a legitimate `pip install -r requirements.txt` could produce an environment Bambuddy fails to start or lint in. CI never caught either, because a fresh runner always resolves to the *top* of every range — only a longer-lived venv resolving lower hits them. **sqlalchemy.** `core/database._create_engine()` passes `pool_size` / `max_overflow` on the SQLite branch. SQLAlchemy **2.0.38** changed the aiosqlite dialect's default pool for file databases from `NullPool` (which rejects both kwargs) to `AsyncAdaptedQueuePool` (which accepts them); on 2.0.0-2.0.37 the module-level `engine = _create_engine()` raises `TypeError: Invalid argument(s) 'pool_size','max_overflow' sent to create_engine()` at import, taking down every SQLite install and the whole test suite (`conftest.py` imports the module). Postgres installs were never affected — `is_sqlite()` is False and the branch is dead. Floor raised to `sqlalchemy>=2.0.38`. **ruff.** The lint job ran a bare `pip install ruff` (always the newest release) while `requirements-dev.txt` said `ruff>=0.8.0`, so CI's linter and a contributor's were routinely *different programs enforcing different rule sets*. A venv holding ruff 0.8.4 reported 32 errors against a tree current ruff calls clean — 30 of them `UP038`, a rule ruff has since **removed** (PEP 604 syntax in `isinstance()` is slower than the tuple form it wanted you to replace). ruff is now pinned exactly (`ruff==0.15.20`) and the CI lint job installs that pin from `requirements-dev.txt`, so local and CI enforce the same rules and `format --check` can't disagree across machines. **Scope.** Packaging + CI only; no application code, no DB migration, no permission, no i18n change. Existing environments should re-run `pip install -U -r requirements.txt -r requirements-dev.txt`.
- **AMS slot with a non-Bambu (no-RFID) spool showed "Empty" instead of "?" (#2527, reporter @NeighborGeek)** — When a spool without a readable RFID tag was loaded, the AMS card showed the slot as **Empty**, while Bambu Studio correctly showed a `?` for an unidentified filament. The reporter's decisive test — swapping the unknown spool between slots and watching "Empty" follow the spool, not the slot — pinned it to slot *content*, not position. Root cause: the authoritative "a spool is physically here" signal is firmware's AMS-level **`tray_exist_bits`** bitmask (what Studio uses to draw the `?`), but Bambuddy inferred emptiness from the *per-tray* `state`/`tray_type`. On the standard AMS (P1-series here, fw 01.09.00.00), a no-RFID spool is reported with an empty `tray_type` and `state=9` — structurally identical to a truly-empty slot at the tray level — so the frontend's `getEmptySlotKind()` classified it as firmware-confirmed-empty and rendered "Empty" rather than the existing `reset` kind that renders `?` ("Spool loaded — slot not configured", #1694). Confirmed from the support bundle: `tray_exist_bits='f'` (all four slots present) with `tray_is_bbl_bits='5'` (only slots 0,2 are Bambu) — slots 1,3 were present-but-non-Bambu, exactly the ones shown Empty. **Fix.** `apply_tray_exist_bits()` — which already parses the bitmask to clear stale fields on absent slots — now also annotates each slot with an authoritative `exists` bool (gated behind a new `annotate_exists` flag so only the printer-card path sets it; the VP bridge leaves it off and the `exists` key never reaches the slicer wire format). `exists` flows through the `AMSTray` schema/serialization to the frontend, where `getEmptySlotKind()` uses it: `exists === true` + no `tray_type` → `?` (present, unconfigured), `exists === false` → Empty, and `exists` absent → the previous `state=9/10` heuristic (so AMS-HT and missing-bitmask paths are unchanged). This is why the bug never reproduced on H2D or X1C — their firmware already reports present-unknown slots with a non-9 `state`, so they fell through to `reset`/`?`; with the fix they take the same path via `exists` and are unaffected. Supersedes the closed #1838. **Tests.** Backend: 3 helper cases (present/absent slots annotated, a present-no-`tray_type` slot marked `exists=true` and left uncleared, and `annotate_exists` off keeps the wire dict clean). Frontend: 1 `AmsUnitCard` case (a `state=9` slot with `exists=true` renders `?`, while `exists=false` still renders "Empty"). Full `test_bambu_mqtt` + VP-bridge suites 384/384 and the AMS/printer/VP backend selection green; `ruff` clean; `npm run build` + ESLint clean; `AmsUnitCard`/`PrintersPage` vitest green. **Scope.** No DB migration, no new permission, no i18n change; VP slicer-facing wire format unchanged.
- **Postgres→SQLite backup dropped NOT NULL / DEFAULT / FK / UNIQUE, causing NULLs after restore (#2526, reporter @bmorrison9)** — On a PostgreSQL install, `create_backup_zip()` exports a portable SQLite copy of the database so backups can move between engines. It rebuilt each table with only column name + type + primary key — the code's own comment admitted "simplified — just column names and types" — dropping `NOT NULL`, `server_default`/`DEFAULT`, foreign keys, and unique constraints. When such a backup is restored onto a SQLite install, `restore_backup()` page-copies the file straight onto the live database (`sqlite3.Connection.backup()`), so the stripped-down schema becomes the *running* database; the post-restore `init_db()` can't repair it because `create_all()` is `CREATE TABLE IF NOT EXISTS` and never alters existing tables. The reporter root-caused it precisely: `SpoolBuddyDevice.created_at` is `server_default=func.now()`, so SQLAlchemy omits the column on INSERT and relies on the DB default — but with no `DEFAULT` clause the row got a bare `NULL`, which then failed Pydantic validation (`DeviceResponse.created_at`) on the next read and 500'd. Every `server_default` column across the schema was exposed the same way, and the FK/unique loss followed from the same simplified CREATE TABLE. **Fix.** The PostgreSQL branch now builds the portable SQLite schema with `Base.metadata.create_all()` against a SQLite engine — the exact DDL a native SQLite install gets — instead of the hand-rolled loop. That emits `NOT NULL`, `DEFAULT` (`server_default=func.now()` → `DEFAULT (CURRENT_TIMESTAMP)`), foreign keys, unique constraints, and indexes, so a Postgres→SQLite restore reproduces the same effective schema a fresh SQLite install would have. The data-export insert path is unchanged, and the `#1333` OIDC-icon guard is preserved automatically — `LargeBinary` renders as `BLOB` under the real DDL — which let the now-redundant `_sqlalchemy_type_to_sqlite_type()` type-mapping helper be removed. This fixes newly-created backups; a backup taken with an older build still carries the degraded schema, so re-take backups after upgrading. **Tests.** The `#1333` type-mapping unit tests were replaced with three that inspect the *real* backup schema (`metadata.create_all` on SQLite, read back via `sqlite_master`/`PRAGMA table_info`): the OIDC `icon_data` column is `BLOB` (#1333), `spoolbuddy_devices.created_at` keeps its `CURRENT_TIMESTAMP` DEFAULT (#2526), and a NOT NULL non-PK column stays NOT NULL. Full backup/restore suite (`test_settings_api`, `test_security`, Postgres-restore-cascade, SQLite-WAL-safety, OIDC-blob-roundtrip) 136/136 green; `ruff` clean. **Scope.** Backend-only, PostgreSQL-source backups. No DB migration, no new permission, no i18n change.
- **"Store on external storage" diagnostic reported an unresolvable fail on P1S/P1P (#2524, reporter @gregspatrick)** — Install-step-4's `external_storage` check hard-**failed** for a P1S even though there is no reachable UI anywhere — Bambu Studio, OrcaSlicer, Handy, or the printer (P1S has no screen) — that can turn the option on. The reporter root-caused it precisely: `has_external_storage()` returns True for the P1S (it does have a MicroSD slot), so the check proceeds to read `state.store_to_sdcard` (MQTT `home_flag` bit 11), which is stuck `False`. The toggle only renders in Studio when the printer publishes `support_save_remote_print_file_to_storage`, and current P1-series firmware (through 01.10.00.00) never does — Bambu's own storage-cache wiki lists P1 Series as "Not Supported". So the user was shown a red fail they could never clear. **Fix.** New `NO_REMOTE_STORAGE_TOGGLE_MODELS` set (P1S, P1P) + `has_remote_storage_toggle()` helper, distinct from the no-slot `NO_EXTERNAL_STORAGE_MODELS` used for A1/A1 Mini (the P1S genuinely *has* a slot — conflating the two would be wrong). When a model has a slot but no reachable toggle and `store_to_sdcard` is False, the diagnostic now emits `skip` with `params={"reason": "unsupported_model"}` instead of `fail`, and the overall result no longer escalates to "problems" for it. A P1S that somehow reports the option *on* still passes. The gate is model-scoped and default-open, so X1/P2S/H2-class printers — where the toggle is reachable and the fail is actionable — are unaffected; if a future P1 firmware surfaces the capability, drop the model from the set and the check reactivates. The frontend `DiagnosticChecklist` picks a reason-specific message variant (`external_storage.skip_unsupported_model`) when a check carries a `reason`, falling back to the plain per-status text otherwise — so instead of the misleading generic "needs a live MQTT connection" skip line, P1 users see an accurate explanation that the option can't be enabled on current firmware and archived prints may lack thumbnails/metadata until Bambu adds support. **i18n.** 1 new key translated across all 11 locales; parity green at 5580 leaves each. **Tests.** Backend: 3 diagnostic cases (P1S/P1P → skip with the reason param, overall stays "ok"; P1S with the option on still passes) + 3 helper cases in `test_printer_models.py` (P1-series false, other models/unknown/empty true). Frontend: 2 `ConnectionDiagnosticModal` cases (reason variant renders and suppresses the generic text; no-reason falls back). Full diagnostic + model suites green; `ruff` clean; `npm run build` + ESLint clean. **Scope.** No DB migration, no new permission.
+6 -1
View File
@@ -186,7 +186,12 @@ async def update_tag(
)
@router.delete("/{tag_id}", status_code=204)
# response_model=None is load-bearing under `from __future__ import annotations`:
# the `-> None` return annotation reaches FastAPI as the string "None", which it
# resolves to NoneType — a truthy class — and then asserts a 204 may carry no
# response body. fastapi >= 0.116 special-cases NoneType; on the 0.109-0.115
# releases requirements.txt still allows, the app fails at import without this.
@router.delete("/{tag_id}", status_code=204, response_model=None)
async def delete_tag(
tag_id: int,
db: AsyncSession = Depends(get_db),