Stop offering AMS slots as places to store a spool

    The Storage Location dropdown listed entries like "H2D-1 - AMS A1" next to
    real locations, and they could not be got rid of.

    They were never locations. Bambuddy used to record which slot a spool was
    loaded into by writing that string into Spoolman's location field, and the
    writer went away when Storage Location became something the user picks --
    but the strings stayed on people's Spoolman spools, and the location sync
    imports every distinct one it finds, so they have been coming back in
    through the front door ever since. A printer slot is where a spool is
    loaded, not where it is put away, and slot assignments already track the
    first.

    Deleting one by hand did not work either, which is what made this a dead
    end rather than an annoyance: the delete route refuses a location that has
    spools, and in Spoolman mode it counts them by matching that same string,
    so every marker still sitting on a loaded spool answered 409 -- and the two
    that were empty were back on the next sync a minute later.

    The import now skips them and a one-shot migration clears the ones already
    in the catalogue. The shape is defined once and used by both: an optional
    printer-name prefix followed by AMS A1, AMS-HT A1 or External Spool, which
    is exactly what convert_ams_slot_to_location produced. It stays narrow on
    purpose -- "AMS Drybox" and "Spare AMS trays" are somebody's shelf, and
    anything the filter swallowed would be a place they could no longer file a
    spool under -- so both directions are pinned by tests.

    A row is only removed when no spool in this database points at it, by id or
    by legacy free-text name, so an internal-mode user who has deliberately
    filed spools under such a name keeps it. Spools in Spoolman are neither
    consulted nor touched: their location strings are the user's data on the
    user's server, and one that still reads "H2D-1 - AMS A1" in the inventory
    list is telling the truth about what Spoolman holds. It simply stops being
    offered as a destination.

    Verified on a live Postgres instance carrying the reported symptom: 13
    locations down to 3, all ten markers removed, the two real shelves and one
    hand-typed Spoolman name left alone.

backend/app/core/database.py

@@ -4432,6 +4432,81 @@ async def run_migrations(conn):
     # whatever this database actually holds.
     await _migrate_repair_rfid_core_weight(conn)
 
+    # Migration: drop the AMS slot markers an older Bambuddy wrote into
+    # Spoolman and the location sync then imported as storage locations.
+    await _migrate_drop_ams_slot_locations(conn)
+
+
+async def _migrate_drop_ams_slot_locations(conn) -> None:
+    """Remove imported AMS slot markers from the storage-location catalogue.
+
+    Bambuddy used to record which slot a spool was loaded into by writing
+    "<printer> - AMS A1" into Spoolman's ``location`` field. That writer went
+    away when Storage Location became something the user picks (#1114), but the
+    strings stayed on people's Spoolman spools, and
+    ``sync_locations_from_spoolman`` imported every distinct one -- so a printer
+    slot turned up in the Storage Location dropdown as somewhere to put a spool
+    away. Worse, they could not be cleared by hand: the delete route refuses a
+    location that has spools, and in Spoolman mode it counts them by matching
+    that same string, so every marker still on a loaded spool answered 409.
+
+    The import now skips them (``is_ams_slot_location``); this clears the ones
+    already in the catalogue. A row is only deleted when no spool in this
+    database points at it -- neither by ``location_id`` nor by a legacy
+    free-text ``storage_location`` -- so an internal-mode user who has
+    deliberately filed spools under such a name keeps it, dropdown entry and
+    all.
+
+    Spools in Spoolman are not consulted and not touched: their ``location``
+    strings are the user's data on the user's server, and one that still reads
+    "H2D-1 - AMS A1" in the inventory list is telling the truth about what
+    Spoolman holds. It simply stops being offered as a destination, which is
+    the whole point -- those are exactly the markers this cleans up.
+    """
+    from sqlalchemy import text
+
+    from backend.app.services.location_service import is_ams_slot_location, location_name_key
+
+    flag = "_cleanup_ams_slot_locations_done"
+
+    async with conn.begin_nested():
+        already = (
+            await conn.execute(text('SELECT value FROM settings WHERE "key" = :k'), {"k": flag})
+        ).scalar_one_or_none()
+        if already:
+            return
+
+        rows = (await conn.execute(text("SELECT id, name FROM locations"))).fetchall()
+        removed = []
+        for row in rows:
+            if not is_ams_slot_location(row.name):
+                continue
+            in_use = (
+                await conn.execute(
+                    text(
+                        "SELECT COUNT(*) FROM spool WHERE location_id = :id "
+                        "OR LOWER(TRIM(COALESCE(storage_location, ''))) = :key"
+                    ),
+                    {"id": row.id, "key": location_name_key(row.name)},
+                )
+            ).scalar_one()
+            if in_use:
+                continue
+            await conn.execute(text("DELETE FROM locations WHERE id = :id"), {"id": row.id})
+            removed.append(row.name)
+
+        if removed:
+            logger.info(
+                "Removed %d AMS slot marker(s) from the storage-location catalogue: %s",
+                len(removed),
+                ", ".join(sorted(removed)),
+            )
+
+        await conn.execute(
+            text('INSERT INTO settings ("key", value) VALUES (:k, :v)'),
+            {"k": flag, "v": "true"},
+        )
+
 
 async def _migrate_repair_rfid_core_weight(conn) -> None:
     """Correct the tare of RFID-added spools that took the wrong catalogue row (#2909).

backend/app/services/location_service.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import logging
+import re
 import time
 from dataclasses import dataclass
 
@@ -18,6 +19,24 @@ logger = logging.getLogger(__name__)
 
 DUPLICATE_LOCATION_NAME = "A location with this name already exists"
 
+# AMS residency markers, not storage locations. Bambuddy used to write the slot
+# a spool was loaded into -- "<printer> - AMS A1", the shape
+# `SpoolmanClient.convert_ams_slot_to_location` still produces -- straight into
+# Spoolman's `location` field. That writer went away when Storage Location
+# became a place the user chooses (#1114), but the strings survive on people's
+# Spoolman spools, and importing them offers a printer slot as somewhere to put
+# a spool away. A slot is where a spool is loaded, not where it is stored, and
+# Bambuddy tracks that separately through slot assignments.
+_AMS_SLOT_LOCATION_RE = re.compile(
+    r"^(?:.+\s-\s)?(?:AMS[- ]HT [A-Z]\d+|AMS [A-Z]\d+|External Spool)$",
+    re.IGNORECASE,
+)
+
+
+def is_ams_slot_location(name: str) -> bool:
+    """True when a location string names a printer slot rather than a storage place."""
+    return bool(_AMS_SLOT_LOCATION_RE.match(name.strip()))
+
 
 def normalize_location_name(name: str) -> str:
     trimmed = name.strip()
@@ -281,6 +300,9 @@ async def sync_locations_from_spoolman(db: AsyncSession, client) -> bool:
         name = (raw or "").strip()
         if not name:
             continue
+        if is_ams_slot_location(name):
+            logger.debug("Skipping AMS slot marker %r from the Spoolman location import", name)
+            continue
         key = location_name_key(name)
         if key not in by_key:
             by_key[key] = name

backend/tests/unit/test_ams_slot_location_cleanup.py

@@ -0,0 +1,148 @@
+"""Cleanup of AMS slot markers imported into the storage-location catalogue.
+
+Bambuddy used to write the slot a spool was loaded into -- "<printer> - AMS A1"
+-- into Spoolman's ``location`` field, and the location sync then imported every
+distinct one as a storage location. ``_migrate_drop_ams_slot_locations`` clears
+the rows that already landed; the import side is covered in
+``test_location_service.py``.
+"""
+
+import pytest
+from sqlalchemy import text
+from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
+
+import backend.app.models  # noqa: F401 - populate Base.metadata
+from backend.app.core.database import Base, _migrate_drop_ams_slot_locations
+from backend.app.models.location import Location
+from backend.app.models.spool import Spool
+from backend.app.services.location_service import assign_location_name
+
+FLAG = "_cleanup_ams_slot_locations_done"
+
+
+@pytest.fixture
+async def engine(tmp_path):
+    eng = create_async_engine(f"sqlite+aiosqlite:///{tmp_path}/t.db")
+    async with eng.begin() as conn:
+        await conn.run_sync(Base.metadata.create_all)
+    try:
+        yield eng
+    finally:
+        await eng.dispose()
+
+
+def _location(name: str) -> Location:
+    loc = Location()
+    assign_location_name(loc, name)
+    return loc
+
+
+async def _names(db) -> set[str]:
+    return {r[0] for r in (await db.execute(text("SELECT name FROM locations"))).fetchall()}
+
+
+async def _run(engine):
+    async with engine.begin() as conn:
+        await _migrate_drop_ams_slot_locations(conn)
+
+
+@pytest.mark.asyncio
+async def test_removes_the_slot_markers_and_keeps_real_locations(engine):
+    sm = async_sessionmaker(engine, expire_on_commit=False)
+    async with sm() as db:
+        db.add_all(
+            [
+                _location("H2D-1 - AMS A1"),
+                _location("H2D-1 - AMS C3"),
+                _location("X1C-2 - AMS-HT A1"),
+                _location("P1S - External Spool"),
+                _location("Drybox 1"),
+                _location("Shelf A"),
+            ]
+        )
+        await db.commit()
+
+    await _run(engine)
+
+    async with sm() as db:
+        assert await _names(db) == {"Drybox 1", "Shelf A"}
+
+
+@pytest.mark.asyncio
+async def test_keeps_a_marker_a_spool_is_actually_filed_under(engine):
+    """Deleting it would strand the spool's location, and someone who has
+    deliberately filed spools under that name meant it."""
+    sm = async_sessionmaker(engine, expire_on_commit=False)
+    async with sm() as db:
+        loc = _location("H2D-1 - AMS A1")
+        db.add(loc)
+        await db.flush()
+        db.add(
+            Spool(
+                material="PLA",
+                label_weight=1000,
+                location_id=loc.id,
+                storage_location="H2D-1 - AMS A1",
+            )
+        )
+        await db.commit()
+
+    await _run(engine)
+
+    async with sm() as db:
+        assert await _names(db) == {"H2D-1 - AMS A1"}
+
+
+@pytest.mark.asyncio
+async def test_keeps_a_marker_a_legacy_free_text_spool_still_names(engine):
+    """Rows predating the location catalogue carry the name without the FK, and
+    the rename cascade still matches them on it."""
+    sm = async_sessionmaker(engine, expire_on_commit=False)
+    async with sm() as db:
+        db.add(_location("H2D-1 - AMS A1"))
+        await db.flush()
+        # Whitespace and case around the name are the legacy shape the rename
+        # cascade already has to cope with, so the guard has to match it too.
+        db.add(Spool(material="PLA", label_weight=1000, storage_location="  h2d-1 - ams a1 "))
+        await db.commit()
+
+    await _run(engine)
+
+    async with sm() as db:
+        assert await _names(db) == {"H2D-1 - AMS A1"}
+
+
+@pytest.mark.asyncio
+async def test_runs_exactly_once(engine):
+    """A location the user creates afterwards is theirs, whatever it is named."""
+    sm = async_sessionmaker(engine, expire_on_commit=False)
+    async with sm() as db:
+        db.add(_location("H2D-1 - AMS A1"))
+        await db.commit()
+
+    await _run(engine)
+
+    async with sm() as db:
+        db.add(_location("H2D-1 - AMS B2"))
+        await db.commit()
+
+    await _run(engine)
+
+    async with sm() as db:
+        assert await _names(db) == {"H2D-1 - AMS B2"}
+
+
+@pytest.mark.asyncio
+async def test_marks_itself_done_on_an_install_with_nothing_to_remove(engine):
+    """Otherwise the whole catalogue is rescanned on every boot for ever."""
+    sm = async_sessionmaker(engine, expire_on_commit=False)
+    async with sm() as db:
+        db.add(_location("Drybox 1"))
+        await db.commit()
+
+    await _run(engine)
+
+    async with sm() as db:
+        done = (await db.execute(text('SELECT value FROM settings WHERE "key" = :k'), {"k": FLAG})).scalar_one_or_none()
+        assert done == "true"
+        assert await _names(db) == {"Drybox 1"}

backend/tests/unit/test_location_service.py

@@ -9,6 +9,7 @@ from backend.app.services.location_service import (
     assign_location_name,
     enrich_spool_dicts_with_location_id,
     get_location_by_name,
+    is_ams_slot_location,
     location_name_key,
     prepare_internal_spool_payload,
     rename_location,
@@ -217,3 +218,74 @@ async def test_sync_locations_from_spoolman_handles_dict_payload(db_session: Asy
 
     cabinet = await get_location_by_name(db_session, "Cabinet 3")
     assert cabinet is not None
+
+
+class TestIsAmsSlotLocation:
+    """A printer slot is where a spool is loaded, not where it is stored."""
+
+    @pytest.mark.parametrize(
+        "name",
+        [
+            "H2D-1 - AMS A1",
+            "X1C-2 - AMS C3",
+            "P1S - AMS-HT A1",
+            "H2D-1 - AMS HT B1",
+            "AMS A1",
+            "AMS-HT A1",
+            "External Spool",
+            "H2D-1 - External Spool",
+            "h2d-1 - ams a1",
+            "  H2D-1 - AMS A1  ",
+        ],
+    )
+    def test_slot_markers_are_recognised(self, name):
+        assert is_ams_slot_location(name) is True
+
+    @pytest.mark.parametrize(
+        "name",
+        [
+            "Drybox 1",
+            "Shelf A",
+            "AMS Drybox",
+            "Spare AMS trays",
+            "Locker - Top",
+            "AMS A1 spares",
+            "dadadad",
+        ],
+    )
+    def test_real_storage_locations_are_kept(self, name):
+        """The filter has to be narrow: anything it swallows is a place the user
+        can no longer file a spool under."""
+        assert is_ams_slot_location(name) is False
+
+
+@pytest.mark.asyncio
+async def test_sync_locations_from_spoolman_skips_ams_slot_markers(db_session: AsyncSession):
+    """Bambuddy used to write the loaded slot into Spoolman's `location` field.
+    Importing those back offered a printer slot as a storage location, and in
+    Spoolman mode they could not even be deleted -- the delete route counts
+    spools by that same string and answered 409."""
+
+    class FakeClient:
+        async def get_distinct_locations(self):
+            return ["H2D-1 - AMS A1", "X1C-2 - AMS A1", "H2D-1 - External Spool", "Drybox 1"]
+
+    changed = await sync_locations_from_spoolman(db_session, FakeClient())
+    assert changed is True
+    await db_session.commit()
+
+    assert await get_location_by_name(db_session, "Drybox 1") is not None
+    for marker in ("H2D-1 - AMS A1", "X1C-2 - AMS A1", "H2D-1 - External Spool"):
+        assert await get_location_by_name(db_session, marker) is None
+
+
+@pytest.mark.asyncio
+async def test_sync_locations_from_spoolman_reports_no_change_when_only_markers(db_session: AsyncSession):
+    """`changed` drives the caller's commit — claiming a change for rows that
+    were all filtered out would open a write transaction on every poll."""
+
+    class FakeClient:
+        async def get_distinct_locations(self):
+            return ["H2D-1 - AMS A1", "H2D-1 - AMS A2"]
+
+    assert await sync_locations_from_spoolman(db_session, FakeClient()) is False