mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
Fix external-folder scan deleting README.md records; index markdown (#2520)
.md was missing from _SCANNABLE_EXTENSIONS, so scanning an external folder skipped markdown during the walk and the cleanup pass deleted its LibraryFile row (assuming it was gone from disk), 404ing the Folder Readme panel. Add .md to the scannable set so pre-existing markdown is indexed, and gate cleanup deletion on actual disk presence rather than absence from the extension-filtered found_paths, so any non-scannable upload still on disk survives a scan.
This commit is contained in:
maziggy
@@ -37,6 +37,7 @@ 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
|
||||
- **Scanning an external folder no longer deletes the README.md record (and now indexes pre-existing markdown) (#2520, reporter @zumik3-del)** — The Folder Readme panel (#1268) worked for a `README.md` uploaded through Bambuddy's Upload button, but clicking **Scan External Folder** afterwards made the panel vanish. The reporter root-caused it precisely: `.md` was absent from `_SCANNABLE_EXTENSIONS` (`backend/app/api/routes/library.py:1342`), so the `os.walk` pass skipped markdown files (`:1619`) and never added them to `found_paths` — and the end-of-scan cleanup loop deleted any existing external `LibraryFile` whose path wasn't in `found_paths` (`:1724`), assuming it had been removed from disk. The md file was untouched on disk; only its DB row was destroyed, after which the readme endpoint (`GET /folders/{id}/readme`, which matches `filename LIKE '%.md'`) 404'd and the panel hid. **Two-part fix.** **(1)** Added `.md` to `_SCANNABLE_EXTENSIONS`, so the scan now *indexes* markdown that already exists on disk — markdown dropped in by external tools or copied in manually (feature-request item 1 in the same issue) is picked up and shown, and an uploaded md file is re-found instead of being treated as deleted. `.md` classifies as `file_type="md"` and hits none of the 3mf/gcode/image thumbnail gates, so it just creates a plain record. **(2)** Hardened the cleanup loop to gate deletion on actual disk presence (`path_str not in found_paths and not os.path.exists(path_str)`) rather than mere absence from the extension-filtered `found_paths`. This closes the broader class the reporter flagged: *any* file the upload path admitted whose extension is outside the scannable set (e.g. a `.txt` note) would previously be purged from the DB on the next scan even though it still exists on disk — now such records survive, while genuinely-deleted files (absent from disk) are still cleaned up. **Tests.** 3 new cases in `test_external_folders_api.py::TestExternalFolderScan`: a pre-existing `README.md` on disk is discovered by scan and served by the readme endpoint; an uploaded `README.md` survives a scan (`removed == 0`) and the panel still resolves it — the exact reported bug; a non-scannable `.txt` upload survives a scan via the disk-presence guard. Full suite 39/39 green; `ruff check backend/` clean. **Scope.** Backend-only. No DB migration, no new permission, no i18n change. Item 2 of the issue (the readme panel taking up too much vertical space / needing a page scroll or collapsible sidebar) is a separate File Manager frontend change, tracked as a follow-up on the same issue.
|
||||
- **Nozzle sizes other than 0.4mm now fully supported in AMS Slot config + pre-dispatch guard (#1899, reporter @TheUltimateC0der; also hit by @icaisolutionsb2b-hub on X2D)** — On an H2S (or any printer) with a 0.6mm nozzle installed, the **Configure AMS Slot** picker only ever offered 0.4mm filament presets, so trays couldn't be set to the profile that matched the slice, and dispatching the 0.6-sliced job made the printer bail out with the cryptic HMS `_8012` "Failed to get AMS mapping table". Two distinct gaps. **(1) The slot picker was hardwired to 0.4mm.** `ConfigureAmsSlotModal` takes a `nozzleDiameter` prop that defaults to `'0.4'` (`ConfigureAmsSlotModal.tsx:287`) and drives both the local-preset compatibility filter (it builds `"Bambu Lab H2S 0.4 nozzle"` and rejects imported 0.6 presets whose `compatible_printers` lists "…0.6 nozzle") and the K-profile query. Neither call site — `PrintersPage.tsx` nor the SpoolBuddy kiosk's `SpoolBuddyAmsPage.tsx` — ever passed the prop, so the modal assumed 0.4 regardless of the hardware. The real installed diameter was already in scope on the printer status (`status.nozzles[0].nozzle_diameter`, the same field that renders the "• 0.6mm" badge on the card). **Fix:** new `resolveSlotNozzleDiameter(status, amsId)` helper in `utils/amsHelpers.ts` reads the installed nozzle for a given AMS — on dual-nozzle printers (H2D) it resolves the specific nozzle feeding that AMS via `ams_extruder_map[amsId] → nozzles[idx]`, on single-nozzle printers it falls back to the primary nozzle, and it returns `undefined` when the printer hasn't reported nozzle hardware yet so the modal keeps its 0.4 default. Both call sites now pass `nozzleDiameter={resolveSlotNozzleDiameter(status, slot.amsId)}`, so the picker filters presets by the nozzle actually on the machine. **(2) No pre-dispatch validation of nozzle size.** Nothing in the dispatch path (`_compute_ams_mapping_for_printer` / `_match_filaments_to_slots`) ever compared the sliced nozzle diameter against the installed nozzle — the AMS mapping matches on `tray_info_idx` → colour → type with a hard filter only on extruder id, never diameter — so Bambuddy would ship a mapping the firmware then rejects with `_8012` (or `0500_4038`), leaving the user staring at a printer-side error with no explanation. **Fix:** a nozzle-mismatch guard in `_start_print` (backend), placed before preheat and upload so no time is wasted, compares `archive.nozzle_diameter` (parsed from the sliced 3MF's `slice_info`; `None` when the slice doesn't declare it) against the printer's reported nozzles via two pure helpers `_installed_nozzle_diameters()` and `_nozzle_mismatch_message()`. On a positive mismatch it fails the queue item with an actionable message — "File sliced for a 0.6mm nozzle, but the printer has 0.4mm installed. Re-slice for the installed nozzle, or install the matching nozzle before printing." — and fires the same failed-notification + WS event as other dispatch failures. **Fail-safe by construction:** it blocks ONLY on a positive mismatch — when the slice carries no nozzle diameter, or the printer hasn't reported its nozzles, the guard is a no-op and dispatch proceeds exactly as before; on dual-nozzle printers a match against EITHER installed nozzle passes (a 0.6 slice is fine if one of the two hotends is a 0.6). The 0.05mm tolerance absorbs float noise while staying well inside the 0.2mm gap between adjacent nozzle sizes. **Tests.** Frontend: 7 cases in `resolveSlotNozzleDiameter.test.ts` (null/empty status, single-nozzle, dual-nozzle per-AMS resolution, fallbacks). Backend: 15 cases in `test_scheduler_nozzle_mismatch.py` — 5 for `_installed_nozzle_diameters` (parse, empty-default stub, unparseable/zero, dual-nozzle), 8 for `_nozzle_mismatch_message` (block/pass, float tolerance, dual-nozzle either-match, both fail-safe None paths, adjacent-size discrimination), and 2 end-to-end `_start_print` cases proving a mismatch fails the item *before* upload/start_print and a match lets dispatch proceed. Existing scheduler suites (cleanup-library, ams-mapping, cancel-race, preheat — 118 tests) stay green, which also proves the guard is a transparent no-op on the existing archive-without-nozzle path. `npm run build`, ESLint, `ruff check backend/` all clean. **Scope.** No DB migration, no new permission, no new i18n key (the failure message rides the existing `error_message` surface already rendered on failed queue items). Frontend picker change + backend guard only.
|
||||
- **"Remember Me" appeared broken — an authenticated visit to `/login` rendered the login form instead of redirecting (#1889, reporter @superdong69)** — Users with a perfectly valid, persisted session reported that Bambuddy "never stays logged in": they log in with Remember Me, come back later, and are met with the login form again. The reporter did the legwork and traced it to routing, not session persistence: `frontend/src/pages/LoginPage.tsx` destructured only `const { login, loginWithToken } = useAuth()` and never looked at the authenticated state, so the `/login` route (rendered unwrapped in `App.tsx` — `ProtectedRoute` only guards the *other* direction, unauthenticated → `/login`) showed the credentials step even when the token was live. On that same page load the app's own bootstrap sends `GET /api/v1/auth/me` with the Bearer token and gets a 200 with the full user object — the session is fully alive; only the view is wrong. **Why it's easy to hit and self-reinforcing.** After a few visits the browser address bar autocompletes the origin to its most-visited path, which becomes `/login`, so every subsequent visit lands on the form and the illusion of "logged out" compounds. Navigating to `/` instead lands on the dashboard, logged in, no credentials asked — which is also why it can't be reproduced by testing `/` directly. **Fix.** `LoginPage` now also reads `user` and `loading` from the auth context and, in a `useEffect`, redirects an already-authenticated visitor with `navigate('/', { replace: true })` once the auth check has settled. The effect is gated on `step === 'credentials'` so it never interrupts the 2FA step or the OIDC-callback branch, both of which perform their own `navigate()` after `loginWithToken`. It redirects to `/` rather than `resolvePostLoginRedirect()` so it can't consume the OIDC redirect stash — an already-authenticated direct visit has no pending redirect to honour. **Tests.** 2 new cases in `LoginPage.test.tsx` (`authenticated redirect (#1889)`): a live session (token set + `/auth/me` → 200) redirects to `/` with `replace: true`; an unauthenticated visit renders the Sign in form and does not redirect. Existing 29 LoginPage cases stay green; `npm run build` and ESLint clean. **Scope.** Frontend-only, routing layer. No backend change, no DB migration, no new permission, no new i18n key. Note this is the routing facet of #1889; the separate token-discard-on-transient-failure hardening in `AuthContext` (don't drop a valid persisted token on a non-401 blip) is already in the tree.
|
||||
- **Multi-nozzle prints no longer collapse all filaments onto one nozzle (#1825, reporter @needo37)** — The single-active-extruder shortcut added in #851 (for #827) at `threemf_tools.py:354` runs `before` the per-filament `group_id` mapping, and fires whenever `extruder_nozzle_stats` reports exactly one extruder as having a nozzle installed. On the H2D / H2D Pro / X2D (2-nozzle) and H2C (3+-nozzle tool-changer), this field is data-driven from the slicer profile's enumerated nozzle volume types — when an HT-AMS or High-Flow nozzle's type isn't enumerated in the slice's profile (common with asymmetric extruder setups, e.g. HT-AMS feeding the right nozzle on an H2D), the slicer emits e.g. `['Standard#1', 'Standard#0']` even though the print genuinely uses both extruders. `sum(active_extruders) == 1` triggered → every filament was force-assigned to `physical_extruder_map[active_idx]`, the authoritative per-filament `group_id` was discarded, and the Filament Mapping panel showed both filaments badged **L** with the auto-match hard filter (`print_scheduler.py` `_compute_ams_mapping_for_printer` ~line 1239) blocking the wrong-nozzle tray as "Type not found". Bug is **parser-side and model-agnostic** — triggers purely on 3MF data shape, not on the attached AMS hardware: regular dual-AMS H2D installs typically slice to `['Standard#1', 'Standard#1']` (sum==2) and never enter the buggy branch, which is why this bug was invisible on the most common dual-AMS setup. Physical nozzle routing was **not** affected — the actual extrude path comes from the sliced gcode + the verbatim `nozzle_mapping` from the project_file (#1780), not from this parse — so the bug surfaced as auto-match failure + wrong L/R badge, not wrong-nozzle extrusion. **Fix.** Gate the single-active shortcut on `len(distinct_group_ids) <= 1` from `slice_info.config`. The slice_info parse is hoisted above the shortcut check (and reused by Priority 1) so the gate adds zero extra I/O. When the slice contains ≥2 distinct group_ids, the shortcut skips and the existing `group_id`-based Priority 1 mapping runs. The gate only **narrows** the shortcut path — it can't widen the buggy collapse onto any previously-working slice. The same condition generalizes to H2C and any future N-nozzle printer for free (no nozzle-count branching). **Tests.** Two new cases in `TestExtractNozzleMappingFrom3MF`: `test_single_active_under_report_with_multi_group_falls_through` pins the #1825 regression (`['Standard#1','Standard#0']` + group_ids `{0,1}` → `{1:1, 2:0}` not `{1:1, 2:1}`); `test_single_active_with_single_group_still_uses_shortcut` preserves the #851 behaviour (same stats + only `group_id=0` → shortcut still fires → `{1:1, 2:1}`). Existing `test_single_active_extruder_maps_all_slots` and `test_two_active_extruders_falls_through` stay green. **Suites.** `pytest -n 30 backend/tests/unit/test_scheduler_ams_mapping.py backend/tests/unit/test_scheduler_filament_deficit.py backend/tests/unit/test_scheduler_filament_override.py backend/tests/unit/test_fallback_archive_mqtt_filament.py backend/tests/integration/test_archives_api.py backend/tests/integration/test_library_api.py` 272/272 green. `ruff check backend/` clean. **Scope.** Backend-only, parse layer. No DB migration. No new permission. No frontend change. The L/R-only badge limitation on 3+-nozzle printers (H2C tool-changer) called out in the report is a separate cosmetic follow-up and not part of this fix.
|
||||
|
||||
@@ -1353,6 +1353,7 @@ _SCANNABLE_EXTENSIONS = {
|
||||
".gif",
|
||||
".webp",
|
||||
".svg",
|
||||
".md",
|
||||
}
|
||||
|
||||
|
||||
@@ -1720,9 +1721,17 @@ async def scan_external_folder(
|
||||
db.add(db_file)
|
||||
added += 1
|
||||
|
||||
# Remove DB entries for files that no longer exist on disk
|
||||
# Remove DB entries for files that no longer exist on disk.
|
||||
#
|
||||
# Gate on actual disk presence, NOT merely absence from found_paths:
|
||||
# found_paths only collects extensions in _SCANNABLE_EXTENSIONS, so a
|
||||
# record for any other file the upload path admitted (e.g. a .md README,
|
||||
# #2520) would otherwise be treated as "deleted from disk" and purged on
|
||||
# every scan even though the file is still there. os.path.exists keeps
|
||||
# such records; genuinely-deleted files (absent from disk) are still
|
||||
# cleaned up. External file_path is the absolute on-disk path.
|
||||
for path_str, db_file in existing_files.items():
|
||||
if path_str not in found_paths:
|
||||
if path_str not in found_paths and not os.path.exists(path_str):
|
||||
# Clean up thumbnail if we generated one
|
||||
if db_file.thumbnail_path:
|
||||
try:
|
||||
|
||||
@@ -295,6 +295,104 @@ class TestExternalFolderScan:
|
||||
assert result["removed"] == 1
|
||||
assert result["added"] == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_scan_indexes_pre_existing_markdown(
|
||||
self, async_client: AsyncClient, db_session, external_folder, external_dir
|
||||
):
|
||||
"""Scan should index a README.md already on disk (#2520 item 1).
|
||||
|
||||
Markdown dropped into the folder by external tools (not the Upload
|
||||
dialog) must be picked up so the Folder Readme panel can show it.
|
||||
"""
|
||||
(external_dir / "README.md").write_text("# Fishing Floats\n\nDescription.")
|
||||
|
||||
response = await async_client.post(f"/api/v1/library/folders/{external_folder['id']}/scan")
|
||||
assert response.status_code == 200
|
||||
# 4 supported files from the fixture + the new README.md
|
||||
assert response.json()["added"] == 5
|
||||
|
||||
response = await async_client.get(f"/api/v1/library/files?folder_id={external_folder['id']}")
|
||||
root_filenames = {f["filename"] for f in response.json()}
|
||||
assert "README.md" in root_filenames
|
||||
|
||||
# Readme panel can now resolve it.
|
||||
response = await async_client.get(f"/api/v1/library/folders/{external_folder['id']}/readme")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["filename"] == "README.md"
|
||||
assert "Fishing Floats" in response.json()["content"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_scan_preserves_uploaded_markdown(self, async_client: AsyncClient, db_session, tmp_path):
|
||||
"""Scanning must not delete an uploaded README.md (#2520 destructive-cleanup bug).
|
||||
|
||||
Before the fix, .md was absent from _SCANNABLE_EXTENSIONS, so an
|
||||
uploaded markdown record was never re-found during the walk and the
|
||||
cleanup pass purged it — the Readme panel then 404'd and hid.
|
||||
"""
|
||||
import io
|
||||
|
||||
writable_dir = tmp_path / "writable"
|
||||
writable_dir.mkdir()
|
||||
response = await async_client.post(
|
||||
"/api/v1/library/folders/external",
|
||||
json={"name": "Writable", "external_path": str(writable_dir), "readonly": False},
|
||||
)
|
||||
folder = response.json()
|
||||
|
||||
upload = await async_client.post(
|
||||
f"/api/v1/library/files?folder_id={folder['id']}",
|
||||
files={"file": ("README.md", io.BytesIO(b"# Model\n\nHello"), "text/markdown")},
|
||||
)
|
||||
assert upload.status_code in (200, 201)
|
||||
|
||||
# Panel works before the scan.
|
||||
readme = await async_client.get(f"/api/v1/library/folders/{folder['id']}/readme")
|
||||
assert readme.status_code == 200
|
||||
|
||||
# The scan that used to nuke the record.
|
||||
scan = await async_client.post(f"/api/v1/library/folders/{folder['id']}/scan")
|
||||
assert scan.status_code == 200
|
||||
assert scan.json()["removed"] == 0
|
||||
|
||||
# Record and panel survive.
|
||||
readme = await async_client.get(f"/api/v1/library/folders/{folder['id']}/readme")
|
||||
assert readme.status_code == 200
|
||||
assert readme.json()["filename"] == "README.md"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_scan_preserves_non_scannable_file_on_disk(self, async_client: AsyncClient, db_session, tmp_path):
|
||||
"""Cleanup must gate on disk presence, not scannable-extension membership (#2520).
|
||||
|
||||
Any uploaded file whose extension is outside _SCANNABLE_EXTENSIONS
|
||||
(here a .txt) stays on disk, so its DB record must survive a scan
|
||||
rather than being treated as deleted.
|
||||
"""
|
||||
import io
|
||||
|
||||
writable_dir = tmp_path / "writable_txt"
|
||||
writable_dir.mkdir()
|
||||
response = await async_client.post(
|
||||
"/api/v1/library/folders/external",
|
||||
json={"name": "Writable Txt", "external_path": str(writable_dir), "readonly": False},
|
||||
)
|
||||
folder = response.json()
|
||||
|
||||
upload = await async_client.post(
|
||||
f"/api/v1/library/files?folder_id={folder['id']}",
|
||||
files={"file": ("notes.txt", io.BytesIO(b"keep me"), "text/plain")},
|
||||
)
|
||||
assert upload.status_code in (200, 201)
|
||||
|
||||
scan = await async_client.post(f"/api/v1/library/folders/{folder['id']}/scan")
|
||||
assert scan.status_code == 200
|
||||
assert scan.json()["removed"] == 0
|
||||
|
||||
files = await async_client.get(f"/api/v1/library/files?folder_id={folder['id']}")
|
||||
assert "notes.txt" in {f["filename"] for f in files.json()}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_scan_non_external_folder_fails(self, async_client: AsyncClient, db_session):
|
||||
|
||||
Reference in New Issue
Block a user