Browse Source

fix(spoolman): allow AMS-HT ams_id range in slot-assignment table (#1274)

  H2C / H2D AMS-HT units report ams_id 128+ (one ams_id per unit, single
  tray), but spoolman_slot_assignments.ck_ams_id_range only admitted 0-7
  and 255. Every attempt to link a Spoolman spool to an AMS-HT slot died
  with `CHECK constraint failed: ck_ams_id_range`. The internal
  spool_assignment table has no such constraint and works fine.

  Widen the formula to (0-7) OR (128-191) OR 255 in the model, the
  CREATE TABLE DDL, and an idempotent in-place migration for existing
  installs (Postgres: DROP/ADD CONSTRAINT; SQLite: detect stale formula
  in sqlite_master, rebuild via _v2 rename pattern).
maziggy 2 weeks ago
parent
commit
af52c4f2ff

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 105 - 2
backend/app/core/database.py

@@ -502,6 +502,103 @@ async def _migrate_update_auto_link_constraint(conn) -> None:
                 raise
 
 
+async def _migrate_widen_spoolman_slot_ams_id_range(conn) -> None:
+    """Widen ck_ams_id_range on spoolman_slot_assignments to admit AMS-HT (#1274).
+
+    Old formula: (ams_id >= 0 AND ams_id <= 7) OR ams_id = 255
+    New formula: (ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255
+
+    The H2C/H2D AMS-HT reports ams_id 128+. The old constraint rejected every
+    AMS-HT slot link with `IntegrityError: CHECK constraint failed: ck_ams_id_range`.
+
+    PostgreSQL: DROP CONSTRAINT IF EXISTS + ADD new formula via _safe_execute.
+    SQLite: table recreation when the old (narrower) formula is detected in
+    sqlite_master. Fresh installs already have the widened constraint from
+    the CREATE TABLE migration above.
+    """
+    from sqlalchemy import text
+
+    _NEW_FORMULA = "(ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255"
+    _CONSTRAINT_NAME = "ck_ams_id_range"
+
+    if not is_sqlite():
+        await _safe_execute(
+            conn,
+            f"ALTER TABLE spoolman_slot_assignments DROP CONSTRAINT IF EXISTS {_CONSTRAINT_NAME}",
+        )
+        await _safe_execute(
+            conn,
+            f"ALTER TABLE spoolman_slot_assignments ADD CONSTRAINT {_CONSTRAINT_NAME} CHECK ({_NEW_FORMULA})",
+        )
+        return
+
+    row = (
+        await conn.execute(
+            text("SELECT sql FROM sqlite_master WHERE type='table' AND name='spoolman_slot_assignments'")
+        )
+    ).fetchone()
+    if not row:
+        return
+    sql = row[0] or ""
+    # Already widened by an earlier run or by the fresh-install CREATE TABLE above.
+    if "ams_id >= 128" in sql:
+        return
+    # Pre-migration table without any CHECK constraint at all → leave alone;
+    # the app-level validation handles correctness and we don't risk a
+    # destructive table rebuild for a constraint that isn't blocking anyone.
+    if "ck_ams_id_range" not in sql and "ams_id <= 7" not in sql:
+        return
+
+    try:
+        async with conn.begin_nested():
+            await conn.execute(text("DROP TABLE IF EXISTS spoolman_slot_assignments_v2"))
+            await conn.execute(
+                text(
+                    "CREATE TABLE spoolman_slot_assignments_v2 ("
+                    "id INTEGER PRIMARY KEY AUTOINCREMENT, "
+                    "printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE, "
+                    f"ams_id INTEGER NOT NULL CHECK ({_NEW_FORMULA}), "
+                    "tray_id INTEGER NOT NULL CHECK (tray_id >= 0 AND tray_id <= 3), "
+                    "spoolman_spool_id INTEGER NOT NULL, "
+                    "assigned_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, "
+                    "CONSTRAINT uq_slot_assignment UNIQUE(printer_id, ams_id, tray_id)"
+                    ")"
+                )
+            )
+            await conn.execute(
+                text(
+                    "INSERT INTO spoolman_slot_assignments_v2 "
+                    "(id, printer_id, ams_id, tray_id, spoolman_spool_id, assigned_at) "
+                    "SELECT id, printer_id, ams_id, tray_id, spoolman_spool_id, assigned_at "
+                    "FROM spoolman_slot_assignments"
+                )
+            )
+            original = (await conn.execute(text("SELECT count(*) FROM spoolman_slot_assignments"))).scalar_one()
+            copied = (await conn.execute(text("SELECT count(*) FROM spoolman_slot_assignments_v2"))).scalar_one()
+            if copied != original:
+                raise RuntimeError(
+                    f"spoolman_slot_assignments migration: row count mismatch after copy "
+                    f"({original} in source, {copied} in copy)"
+                )
+            await conn.execute(text("DROP TABLE spoolman_slot_assignments"))
+            await conn.execute(text("ALTER TABLE spoolman_slot_assignments_v2 RENAME TO spoolman_slot_assignments"))
+            # The index sits on the renamed table; recreate it idempotently
+            # to handle older sqlite versions that don't auto-rename indexes.
+            await conn.execute(
+                text(
+                    "CREATE INDEX IF NOT EXISTS ix_slot_assignment_spool "
+                    "ON spoolman_slot_assignments (spoolman_spool_id)"
+                )
+            )
+    except Exception as exc:
+        logger.error(
+            "spoolman_slot_assignments ck_ams_id_range widening (SQLite table recreation) FAILED: %s",
+            exc,
+            exc_info=True,
+        )
+        raise
+
+
 async def run_migrations(conn):
     """Run all schema migrations and data backfills on startup.
 
@@ -2039,13 +2136,14 @@ async def run_migrations(conn):
     # Migration: Create spoolman_slot_assignments table for local AMS-slot→Spoolman-spool mapping.
     # Replaces the pattern of writing spool.location in Spoolman (which polluted the
     # user-editable storage_location field in the UI).
+    # ck_ams_id_range formula was widened in #1274 to admit AMS-HT (ams_id 128-191).
     await _safe_execute(
         conn,
         """
         CREATE TABLE IF NOT EXISTS spoolman_slot_assignments (
             id INTEGER PRIMARY KEY AUTOINCREMENT,
             printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE,
-            ams_id INTEGER NOT NULL CHECK ((ams_id >= 0 AND ams_id <= 7) OR ams_id = 255),
+            ams_id INTEGER NOT NULL CHECK ((ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255),
             tray_id INTEGER NOT NULL CHECK (tray_id >= 0 AND tray_id <= 3),
             spoolman_spool_id INTEGER NOT NULL,
             assigned_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
@@ -2057,7 +2155,7 @@ async def run_migrations(conn):
         CREATE TABLE IF NOT EXISTS spoolman_slot_assignments (
             id SERIAL PRIMARY KEY,
             printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE,
-            ams_id INTEGER NOT NULL CHECK ((ams_id >= 0 AND ams_id <= 7) OR ams_id = 255),
+            ams_id INTEGER NOT NULL CHECK ((ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255),
             tray_id INTEGER NOT NULL CHECK (tray_id >= 0 AND tray_id <= 3),
             spoolman_spool_id INTEGER NOT NULL,
             assigned_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
@@ -2070,6 +2168,11 @@ async def run_migrations(conn):
         "CREATE INDEX IF NOT EXISTS ix_slot_assignment_spool ON spoolman_slot_assignments (spoolman_spool_id)",
     )
 
+    # Migration: widen ck_ams_id_range on spoolman_slot_assignments to allow
+    # AMS-HT ids (128-191). Existing installs created before #1274 carry the
+    # stale formula which rejects every AMS-HT slot link with a CHECK violation.
+    await _migrate_widen_spoolman_slot_ams_id_range(conn)
+
     # Migration: Create spoolman_k_profile table for K-value calibration profiles linked to Spoolman spools.
     await _safe_execute(
         conn,

+ 8 - 1
backend/app/models/spoolman_slot_assignment.py

@@ -27,7 +27,14 @@ class SpoolmanSlotAssignment(Base):
 
     __table_args__ = (
         UniqueConstraint("printer_id", "ams_id", "tray_id", name="uq_slot_assignment"),
-        CheckConstraint("(ams_id >= 0 AND ams_id <= 7) OR ams_id = 255", name="ck_ams_id_range"),
+        # 0-7: standard AMS units. 128-191: AMS-HT (each unit uses ams_id 128+,
+        # single tray). 255: external / VT tray. Matches the value range the
+        # internal `spool_assignment` table accepts. See #1274 — H2C with
+        # AMS-HT on the left nozzle reports ams_id=128.
+        CheckConstraint(
+            "(ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255",
+            name="ck_ams_id_range",
+        ),
         CheckConstraint("tray_id >= 0 AND tray_id <= 3", name="ck_tray_id_range"),
     )
 

+ 26 - 0
backend/tests/integration/test_spoolman_slot_assignments.py

@@ -105,6 +105,32 @@ class TestAssignSpoolmanSlot:
         assert rows[0]["ams_id"] == 0
         assert rows[0]["tray_id"] == 0
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_assign_accepts_ams_ht_id(self, async_client: AsyncClient, slot_settings, test_printer, mock_client):
+        """#1274: AMS-HT units report ams_id 128+. The pre-fix ck_ams_id_range
+        only allowed 0-7 / 255, so the upsert blew up with `CHECK constraint
+        failed: ck_ams_id_range` and the user couldn't link any spool to the
+        H2C/H2D AMS-HT slot. This guards the widened range from regressing.
+        """
+        response = await async_client.post(
+            "/api/v1/spoolman/inventory/slot-assignments",
+            json={
+                "spoolman_spool_id": 51,
+                "printer_id": test_printer.id,
+                "ams_id": 128,  # AMS-HT on the left nozzle (matches issue's failing INSERT)
+                "tray_id": 0,
+            },
+        )
+
+        assert response.status_code == 200, response.text
+        all_resp = await async_client.get(
+            "/api/v1/spoolman/inventory/slot-assignments/all",
+            params={"printer_id": test_printer.id},
+        )
+        rows = all_resp.json()
+        assert any(r["ams_id"] == 128 and r["spoolman_spool_id"] == 51 for r in rows)
+
     @pytest.mark.asyncio
     @pytest.mark.integration
     async def test_assign_does_not_call_update_spool(

+ 24 - 0
backend/tests/unit/test_spoolman_slot_ddl.py

@@ -75,3 +75,27 @@ class TestSpoolmanSlotDdl:
         table = SpoolmanSlotAssignment.__table__
         check_names = {c.name for c in table.constraints if isinstance(c, CheckConstraint)}
         assert "ck_tray_id_range" in check_names, f"ck_tray_id_range not in ORM check constraints: {check_names}"
+
+    def test_ams_id_check_admits_ams_ht_range(self):
+        """#1274: ck_ams_id_range must accept AMS-HT (ams_id 128-191).
+
+        H2C / H2D AMS-HT units report ams_id starting at 128 (one ams_id per
+        unit, single tray). The pre-fix constraint only allowed 0-7 and 255,
+        so every AMS-HT slot link failed with CHECK violation. Both the ORM
+        formula and the SQLite/Postgres DDL strings must include the new range.
+        """
+        from sqlalchemy import CheckConstraint
+
+        from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment
+
+        table = SpoolmanSlotAssignment.__table__
+        ams_check = next(c for c in table.constraints if isinstance(c, CheckConstraint) and c.name == "ck_ams_id_range")
+        formula = str(ams_check.sqltext)
+        assert "128" in formula and "191" in formula, (
+            f"ck_ams_id_range formula does not cover AMS-HT range: {formula!r}"
+        )
+
+        # Same check at the raw-DDL level so a stale DDL definition can't
+        # silently ship with the loosened ORM formula.
+        ddl = _extract_spoolman_slot_ddl(is_sqlite=True)
+        assert "128" in ddl and "191" in ddl, "AMS-HT range missing from CREATE TABLE DDL"

Some files were not shown because too many files changed in this diff