From ff07a8235854ef7f9fecdbd22c345cdf3cae2f13 Mon Sep 17 00:00:00 2001 From: Kouki Ojima Date: Mon, 28 Sep 2026 16:58:16 +0900 Subject: [PATCH] Keep the consumed-counter reset out of Spoolman's own numbers (issue #2906) (#2939) --- backend/app/api/routes/_spoolman_helpers.py | 72 +++- backend/app/api/routes/spoolman_inventory.py | 21 +- backend/app/services/spoolman.py | 64 +++- .../test_spoolman_inventory_api.py | 41 +- .../test_spoolman_reset_baseline_2906.py | 351 ++++++++++++++++++ 5 files changed, 513 insertions(+), 36 deletions(-) create mode 100644 backend/tests/unit/services/test_spoolman_reset_baseline_2906.py diff --git a/backend/app/api/routes/_spoolman_helpers.py b/backend/app/api/routes/_spoolman_helpers.py index f19dec06b..8269e3e3f 100644 --- a/backend/app/api/routes/_spoolman_helpers.py +++ b/backend/app/api/routes/_spoolman_helpers.py @@ -158,6 +158,51 @@ def _extract_extra_str(extra: dict, key: str) -> str: return decoded if isinstance(decoded, str) else "" +# Spool.extra key holding the consumed-counter baseline: the value Spoolman's +# used_weight had when the user last pressed "Reset usage to 0". Displayed +# consumed is used_weight minus this, which is what internal mode has always +# done with its own weight_used_baseline column (#1644). See #2906. +BAMBU_WEIGHT_USED_BASELINE_KEY = "bambu_weight_used_baseline" + + +def _extract_extra_float(extra: dict, key: str) -> float | None: + """Extract a JSON-encoded number from a Spoolman extra dict. + + Same storage convention as :func:`_extract_extra_str` — Spoolman keeps + extra values as JSON text, so 250.0 is stored as ``"250.0"``. Returns + ``None`` for missing keys, non-finite values, or anything that does not + decode to a number, so the caller can tell "no baseline recorded" from + "a baseline of zero". + """ + raw = extra.get(key) + if raw is None: + return None + if isinstance(raw, (int, float)) and not isinstance(raw, bool): + value = float(raw) + elif isinstance(raw, str): + try: + decoded = json.loads(raw) + except (json.JSONDecodeError, ValueError): + return None + if isinstance(decoded, str): + # Spoolman's extra fields default to field_type "text", which only + # accepts values that decode to a str, so numbers written through + # it arrive as '"263.0"' rather than '263.0'. Both spellings have + # to read back the same, or a baseline written by the reset would + # be invisible to the code that subtracts it. + try: + value = float(decoded) + except ValueError: + return None + elif isinstance(decoded, (int, float)) and not isinstance(decoded, bool): + value = float(decoded) + else: + return None + else: + return None + return value if math.isfinite(value) else None + + def parse_spoolman_multi_colors(filament: dict) -> list[str]: """Spoolman's ``multi_color_hexes`` as a list of bare 6/8-char hex tokens. @@ -251,14 +296,37 @@ def _map_spoolman_spool(spool: dict) -> MappedSpoolFields: # When remaining_weight is unset (legacy spools, or filament linked but # never primed), fall back to the old behaviour: weight_used = # used_weight, baseline = 0. + # + # A "Reset usage to 0" records the then-current used_weight under + # BAMBU_WEIGHT_USED_BASELINE_KEY in spool.extra and touches nothing else, + # so displayed consumed is used_weight minus that. It is folded into the + # baseline here because the frontend's arithmetic is fixed: it always shows + # `weight_used - weight_used_baseline`. The reset used to PATCH Spoolman's + # used_weight to 0 instead, and Spoolman recomputes remaining from initial + # minus used, so the spool jumped back to full (#2906). + # `or 0.0` would collapse None and 0.0, which is the one distinction + # _extract_extra_float exists to preserve; spell the absent case out. + # Clamp the low end too: the write side already refuses a negative + # baseline, and without the same rule here a negative value stored by hand + # would be subtracted, inflating the displayed counter. min() below only + # bounds the other end. + stored = _extract_extra_float(extra, BAMBU_WEIGHT_USED_BASELINE_KEY) + stored_baseline = 0.0 if stored is None else max(0.0, stored) remaining_raw = spool.get("remaining_weight") if remaining_raw is not None: remaining_weight: float = _safe_float(remaining_raw, 0.0) used_weight: float = max(0.0, float(label_weight) - remaining_weight) - weight_used_baseline: float = max(0.0, used_weight - real_used_weight) + weight_used_baseline: float = max(0.0, used_weight - real_used_weight + stored_baseline) else: used_weight = real_used_weight - weight_used_baseline = 0.0 + weight_used_baseline = stored_baseline # already clamped to >= 0 above + # Never let the displayed counter read negative. used_weight can fall below + # the baseline after a reset without anyone touching it: the AMS sync + # writes remaining_weight from the tray percentage, and Spoolman derives + # used_weight from that. The counter then reads 0 until consumption climbs + # past the baseline again. Internal mode behaves the same way, and it is + # far better than before #2906, when the next sync undid the reset outright. + weight_used_baseline = min(weight_used_baseline, used_weight) # Archived state – Spoolman uses a boolean ``archived`` field archived: bool = spool.get("archived", False) diff --git a/backend/app/api/routes/spoolman_inventory.py b/backend/app/api/routes/spoolman_inventory.py index 8f4f43e46..c3663e47c 100644 --- a/backend/app/api/routes/spoolman_inventory.py +++ b/backend/app/api/routes/spoolman_inventory.py @@ -950,18 +950,21 @@ async def reset_spool_consumed_counter( ) -> dict: """Zero the displayed "Total Consumed" counter for a Spoolman spool. - Spoolman doesn't have a native "baseline" field, so the implementation - reaches for the closest equivalent: PATCH `used_weight=0` upstream. - The read mapping in ``_map_spoolman_spool`` then derives Bambuddy's - `weight_used = label - remaining_weight` and `baseline = weight_used - - real_used_weight`, so the Inventory page's `weight_used - baseline` - display lands at 0 while remaining (= label - weight_used) is preserved - — parity with the internal-mode endpoint (#1390, see also + Spoolman has no native baseline field, so the baseline lives in + ``spool.extra`` — the same mechanism internal mode uses for its + ``weight_used_baseline`` column. ``_map_spoolman_spool`` folds it back in, + so the Inventory page's ``weight_used - baseline`` reads 0 while every + native Spoolman field is left exactly as it was. + + It used to PATCH ``used_weight = 0`` upstream, which is not the same thing: + Spoolman recomputes ``remaining_weight`` from initial minus used, so the + spool jumped back to full and the measured remaining filament was gone + (#2906). Parity with the internal-mode endpoint (#1644, #1390, see also ``backend/app/api/routes/inventory.py::reset_spool_consumed_counter``). """ client = await _get_client(db) async with _translate_spoolman_errors(): - spool = await client.reset_spool_usage(spool_id) + spool = await client.reset_spool_consumed_counter(spool_id) try: mapped = _map_spoolman_spool(spool) except ValueError as exc: @@ -1112,7 +1115,7 @@ async def bulk_reset_spool_consumed_counter( for spool_id in spool_ids: try: async with _translate_spoolman_errors(): - await client.reset_spool_usage(spool_id) + await client.reset_spool_consumed_counter(spool_id) reset_count += 1 except HTTPException as exc: logger.warning("Spoolman reset-consumed-counter failed for spool %s: %s", spool_id, exc.detail) diff --git a/backend/app/services/spoolman.py b/backend/app/services/spoolman.py index fa590fb16..919c79631 100644 --- a/backend/app/services/spoolman.py +++ b/backend/app/services/spoolman.py @@ -1,7 +1,9 @@ """Spoolman integration service for syncing AMS filament data.""" import asyncio +import json import logging +import math import weakref from dataclasses import dataclass from datetime import datetime, timezone @@ -15,6 +17,12 @@ logger = logging.getLogger(__name__) BAMBU_RFID_TAG_LENGTH = 32 +# Spool.extra key holding the consumed-counter baseline. Defined here rather +# than imported from the routes package so the client does not depend on it; +# _spoolman_helpers declares the same name for the read side (#2906), and +# test_spoolman_reset_baseline_2906.py asserts the two are equal. +BAMBU_WEIGHT_USED_BASELINE_KEY = "bambu_weight_used_baseline" + @dataclass class SpoolmanSpool: @@ -672,20 +680,50 @@ class SpoolmanClient: ) return response.json() - async def reset_spool_usage(self, spool_id: int) -> dict: - """Reset a spool's used_weight to 0 in Spoolman. + async def reset_spool_consumed_counter(self, spool_id: int) -> dict: + """Zero the displayed consumed counter by recording a baseline in spool.extra. - Used by the per-spool / bulk "Reset usage to 0" actions on the - Inventory page so the Total Consumed stat can be cleared without - touching the rest of the spool's data. + This used to PATCH ``used_weight = 0``, which is not a baseline: Spoolman + recomputes ``remaining_weight`` as initial minus used, so zeroing used + weight put the spool back to full and threw away the measured remaining + filament. The confirmation copy promised the opposite in all thirteen + locales (#2906). + + Writing the baseline into ``extra`` instead touches no native Spoolman + field -- initial, remaining and used all survive -- and is the same + mechanism internal mode has always used for its ``weight_used_baseline`` + column, which is what #1644 asked the two modes to share rather than + approximate. ``_map_spoolman_spool`` folds the stored value back into the + baseline it reports. + + Serialised on the same per-spool lock as ``merge_spool_extra`` so a + concurrent tag or colour write cannot read the extra dict between this + method's fetch and its PATCH and put the old one back. """ - response = await self._request_spool( - "PATCH", - spool_id, - json_body={"used_weight": 0}, - operation="reset-usage", - ) - return response.json() + async with self.extra_lock(spool_id): + current = await self.get_spool(spool_id) # raises on error + used_weight = current.get("used_weight") + try: + baseline = float(used_weight) if used_weight is not None else 0.0 + except (TypeError, ValueError): + baseline = 0.0 + if not math.isfinite(baseline) or baseline < 0: + baseline = 0.0 + merged = { + **(current.get("extra") or {}), + # Stored as a JSON *string*, not a JSON number. Spoolman + # registers an unseen extra key as field_type "text" on first + # write, and its validate_extra_field_value then requires the + # value to decode to a str -- a bare 263.0 is rejected with + # "Value is not a string." and the PATCH 400s, so the reset + # would never land. It has to be right the first time: + # add_or_update_extra_field refuses to change a field's type + # afterwards, so one numeric write would pin the key to text on + # that install permanently. tag, bambu_color_name and both + # slicer keys already store the string form for this reason. + BAMBU_WEIGHT_USED_BASELINE_KEY: json.dumps(str(baseline)), + } + return await self.update_spool_full(spool_id=spool_id, extra=merged) async def update_spool_full( self, @@ -1276,8 +1314,6 @@ class SpoolmanClient: logger.error("Failed to find or create filament for %s", tray.tray_sub_brands) return None - import json - return await self.create_spool( filament_id=filament_id, remaining_weight=remaining, diff --git a/backend/tests/integration/test_spoolman_inventory_api.py b/backend/tests/integration/test_spoolman_inventory_api.py index c7f5ec1e4..aa8792552 100644 --- a/backend/tests/integration/test_spoolman_inventory_api.py +++ b/backend/tests/integration/test_spoolman_inventory_api.py @@ -4,6 +4,7 @@ These tests verify that /api/v1/spoolman/inventory/spools/* correctly translates between Spoolman's data model and Bambuddy's InventorySpool format. """ +import json from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -63,7 +64,27 @@ def mock_spoolman_client(): mock_client.set_spool_archived = AsyncMock( side_effect=lambda spool_id, archived: {**SAMPLE_SPOOLMAN_SPOOL, "archived": archived} ) - mock_client.reset_spool_usage = AsyncMock(return_value={**SAMPLE_SPOOLMAN_SPOOL, "used_weight": 0}) + # The reset records a baseline in spool.extra and touches no native field, + # so the spool comes back with remaining_weight and used_weight unchanged + # (#2906). The previous fixture returned used_weight=0 alongside + # remaining_weight=750.0, which real Spoolman cannot produce -- it + # recomputes remaining from initial minus used, so that response would have + # been 1000.0 and the "remaining unchanged" assertion below would have + # failed. The one check that could have caught the bug was cancelled out by + # the mock. + # + # The baseline is staged as the JSON string '"250.0"', not the JSON number + # '250.0'. Spoolman registers an unseen extra key as field_type "text" and + # then requires the value to decode to a str, so the number form is + # rejected with "Value is not a string." -- staging it here would be the + # same class of mistake as the used_weight=0 above: a value the real server + # cannot hold. + mock_client.reset_spool_consumed_counter = AsyncMock( + return_value={ + **SAMPLE_SPOOLMAN_SPOOL, + "extra": {**SAMPLE_SPOOLMAN_SPOOL["extra"], "bambu_weight_used_baseline": json.dumps("250.0")}, + } + ) mock_client.update_spool_full = AsyncMock(return_value=SAMPLE_SPOOLMAN_SPOOL) mock_client.merge_spool_extra = AsyncMock(return_value=SAMPLE_SPOOLMAN_SPOOL) mock_client.find_or_create_filament = AsyncMock(return_value=7) @@ -654,23 +675,21 @@ class TestSpoolmanInventoryCRUD: ): """POST /spoolman/inventory/spools/{id}/reset-consumed-counter zeroes the displayed counter. - Parity with internal mode (#1390): the InventorySpool response - carries `weight_used = label - remaining` and - `weight_used_baseline = weight_used - real_used_weight`, so the - displayed consumed counter (weight_used - baseline) reads 0 - while remaining (= label - weight_used) preserves Spoolman's - independent remaining_weight field. + Parity with internal mode (#1644): the baseline lives in spool.extra and + `_map_spoolman_spool` folds it into `weight_used_baseline`, so the + displayed consumed counter (weight_used - baseline) reads 0 while every + native Spoolman field — initial, remaining, used — is left alone. """ response = await async_client.post("/api/v1/spoolman/inventory/spools/42/reset-consumed-counter") assert response.status_code == 200 body = response.json() - # Sample spool: label=1000, remaining=750, used_weight=0 after Spoolman reset. + # Sample spool: label=1000, remaining=750, used_weight=250, baseline recorded at 250. assert body["weight_used"] == 250.0, "synthetic weight_used = label - remaining" assert body["weight_used_baseline"] == 250.0, "baseline absorbs the reset" assert body["weight_used"] - body["weight_used_baseline"] == 0, "displayed consumed = 0" assert body["label_weight"] - body["weight_used"] == 750, "remaining unchanged" - mock_spoolman_client.reset_spool_usage.assert_called_once_with(42) + mock_spoolman_client.reset_spool_consumed_counter.assert_called_once_with(42) @pytest.mark.asyncio @pytest.mark.integration @@ -688,7 +707,7 @@ class TestSpoolmanInventoryCRUD: assert response.status_code == 200 assert response.json() == {"reset": 3} - assert mock_spoolman_client.reset_spool_usage.call_count == 3 + assert mock_spoolman_client.reset_spool_consumed_counter.call_count == 3 @pytest.mark.asyncio @pytest.mark.integration @@ -705,7 +724,7 @@ class TestSpoolmanInventoryCRUD: ) assert response.status_code == 400 - mock_spoolman_client.reset_spool_usage.assert_not_called() + mock_spoolman_client.reset_spool_consumed_counter.assert_not_called() @pytest.mark.asyncio @pytest.mark.integration diff --git a/backend/tests/unit/services/test_spoolman_reset_baseline_2906.py b/backend/tests/unit/services/test_spoolman_reset_baseline_2906.py new file mode 100644 index 000000000..0fe1544c7 --- /dev/null +++ b/backend/tests/unit/services/test_spoolman_reset_baseline_2906.py @@ -0,0 +1,351 @@ +"""The consumed-counter reset must not throw away the measured remaining weight (#2906). + +"Reset usage to 0" on a Spoolman-backed spool PATCHed ``used_weight = 0``. +Spoolman derives ``remaining_weight`` as initial minus used, so zeroing the +used weight put the spool back to full — while the confirmation dialog promised, +in all thirteen locales, that "the spool itself, its remaining weight +calculation, and your settings are not changed." + +The fake below is the point of these tests. It recomputes remaining the way a +real Spoolman does instead of accepting whatever it is told, so a reset that +writes ``used_weight`` fails here for the same reason it failed on the reporting +instance. Mocking that rule away is how the original implementation shipped +green: ``test_spoolman_inventory_api.py`` staged ``used_weight: 0`` next to +``remaining_weight: 750.0``, a pair real Spoolman cannot return. +""" + +import json + +import httpx +import pytest + +from backend.app.api.routes._spoolman_helpers import _map_spoolman_spool +from backend.app.services.spoolman import SpoolmanClient + +LABEL_WEIGHT = 1000.0 + + +class FakeSpoolman: + """A Spoolman that applies the three rules a real one applies. + + 1. ``remaining_weight`` is derived, not stored: initial minus used. + 2. An extra key must be registered before it can be written, and an + unregistered one is a 404 on GET so the caller creates it. A field's + type cannot be changed afterwards. + 3. Extra values are validated against the registered type. The default + type is ``text``, which requires the stored JSON to decode to a str. + + Rule 1 is why the reset stopped PATCHing ``used_weight``. Rules 2 and 3 + are why the first version of that fix did not land either: it wrote the + baseline as a JSON number, which a ``text`` field rejects, and the fake + answered every ``/field/spool/`` GET with 200 so neither rule ever ran. + A fake that models the rule that bit last time and not the one biting now + is the same failure as the fixture these tests were written to correct. + """ + + def __init__(self, *, initial: float = LABEL_WEIGHT, used: float = 263.0): + self.spool: dict = { + "id": 42, + "filament": {"id": 7, "name": "PLA Basic", "material": "PLA", "weight": LABEL_WEIGHT}, + "initial_weight": initial, + "used_weight": used, + "remaining_weight": initial - used, + "extra": {}, + } + # What a real instance looks like before this feature runs: the keys + # Bambuddy already writes are registered, all of them text, and + # bambu_weight_used_baseline is not registered at all. + self.fields: dict[str, str] = { + "tag": "text", + "bambu_color_name": "text", + "bambu_slicer_filament_id": "text", + "bambu_slicer_setting_id": "text", + } + self.log: list[str] = [] + + def _validate_extra(self, extra: dict) -> str | None: + """Return Spoolman's error message, or None if the dict is acceptable.""" + for name, raw in extra.items(): + field_type = self.fields.get(name) + if field_type is None: + return f"Unknown extra field {name}." + try: + decoded = json.loads(raw) + except (TypeError, ValueError): + return f"Value for {name} is not valid JSON." + if field_type == "text" and not isinstance(decoded, str): + return "Value is not a string." + return None + + def _recompute(self) -> None: + # With no initial weight there is nothing to derive remaining from, and + # real Spoolman leaves it null rather than inventing one. + initial = self.spool.get("initial_weight") + if initial is None: + self.spool["remaining_weight"] = None + return + self.spool["remaining_weight"] = initial - self.spool["used_weight"] + + async def handler(self, request: httpx.Request) -> httpx.Response: + path = request.url.path.removeprefix("/api/v1") + self.log.append(f"{request.method} {path}") + if path == "/spool/42": + if request.method == "GET": + return httpx.Response(200, json=self.spool) + body = json.loads(request.content) if request.content else {} + if "extra" in body: + error = self._validate_extra(body["extra"]) + if error is not None: + return httpx.Response(400, json={"message": error}) + self.spool.update(body) + # The rule the old fixture mocked away. + self._recompute() + return httpx.Response(200, json=self.spool) + if path.startswith("/field/spool/"): + name = path.rsplit("/", 1)[-1] + if request.method == "GET": + if name not in self.fields: + return httpx.Response(404, json={"message": f"No field {name}."}) + return httpx.Response(200, json={"name": name, "field_type": self.fields[name]}) + if request.method == "POST": + field_type = (json.loads(request.content) if request.content else {}).get("field_type", "text") + existing = self.fields.get(name) + if existing is not None and existing != field_type: + # The reason the storage type has to be right the first + # time: no later release can repair an install that + # registered the key with the wrong type. + return httpx.Response(400, json={"message": "Field type cannot be changed."}) + self.fields[name] = field_type + return httpx.Response(201, json={"name": name, "field_type": field_type}) + raise AssertionError(f"unexpected request {request.method} {path}") + + +@pytest.fixture +def fake(): + return FakeSpoolman() + + +@pytest.fixture +def client(fake): + c = SpoolmanClient("http://localhost:7912") + c._client = httpx.AsyncClient( + transport=httpx.MockTransport(fake.handler), + base_url="http://localhost:7912", + ) + return c + + +@pytest.mark.asyncio +async def test_reset_leaves_the_measured_remaining_weight_alone(client, fake): + """The reported symptom: a spool with 737 g left jumped back to 1000 g.""" + before = fake.spool["remaining_weight"] + + await client.reset_spool_consumed_counter(42) + + assert fake.spool["remaining_weight"] == before == 737.0 + + +@pytest.mark.asyncio +async def test_reset_does_not_touch_any_native_spoolman_field(client, fake): + """initial, remaining and used all survive — the point of using extra. + + The rejected alternative rewrote initial_weight to the current remaining, + which repairs Bambuddy's display by overwriting a field the user owns: + it ratchets down on every reset and Spoolman's own views then show the + spool as full. + """ + await client.reset_spool_consumed_counter(42) + + assert fake.spool["initial_weight"] == 1000.0 + assert fake.spool["used_weight"] == 263.0 + assert fake.spool["remaining_weight"] == 737.0 + + +@pytest.mark.asyncio +async def test_reset_records_the_baseline_in_extra(client, fake): + await client.reset_spool_consumed_counter(42) + + assert fake.spool["extra"]["bambu_weight_used_baseline"] == json.dumps("263.0") + + +@pytest.mark.asyncio +async def test_reset_preserves_other_extra_keys(client, fake): + """The tag lives in the same dict; a reset must not drop it.""" + fake.spool["extra"] = {"tag": '"AABBCCDDEEFF0011AABBCCDDEEFF0011"'} + + await client.reset_spool_consumed_counter(42) + + assert fake.spool["extra"]["tag"] == '"AABBCCDDEEFF0011AABBCCDDEEFF0011"' + assert "bambu_weight_used_baseline" in fake.spool["extra"] + + +@pytest.mark.asyncio +async def test_displayed_consumed_reads_zero_while_remaining_holds(client, fake): + """End to end through the read mapping, which is what the Inventory page uses.""" + await client.reset_spool_consumed_counter(42) + + mapped = _map_spoolman_spool(fake.spool) + + assert mapped["weight_used"] - mapped["weight_used_baseline"] == 0.0, "consumed reads 0" + assert LABEL_WEIGHT - mapped["weight_used"] == 737.0, "remaining still 737 g" + + +@pytest.mark.asyncio +async def test_consumption_after_a_reset_counts_from_the_baseline(client, fake): + """A reset is a baseline, not an erasure: the next print's grams show up.""" + await client.reset_spool_consumed_counter(42) + # 50 g printed afterwards, recorded the way Spoolman's /use endpoint does. + fake.spool["used_weight"] = 313.0 + fake.spool["remaining_weight"] = 687.0 + + mapped = _map_spoolman_spool(fake.spool) + + assert mapped["weight_used"] - mapped["weight_used_baseline"] == 50.0 + assert LABEL_WEIGHT - mapped["weight_used"] == 687.0 + + +@pytest.mark.asyncio +async def test_reset_survives_a_spool_with_no_remaining_weight(client, fake): + """Legacy spools, and spools with a filament linked but never primed, carry + remaining_weight = None. The mapping documents that case; the reset used to + be handed one and PATCH straight through it. + """ + fake.spool["remaining_weight"] = None + fake.spool["initial_weight"] = None + + await client.reset_spool_consumed_counter(42) + mapped = _map_spoolman_spool({**fake.spool, "remaining_weight": None}) + + assert fake.spool["extra"]["bambu_weight_used_baseline"] == json.dumps("263.0") + assert mapped["weight_used"] - mapped["weight_used_baseline"] == 0.0 + + +@pytest.mark.asyncio +async def test_second_reset_moves_the_baseline_forward(client, fake): + """Idempotent in the sense that matters: resetting twice does not compound.""" + await client.reset_spool_consumed_counter(42) + fake.spool["used_weight"] = 313.0 + fake.spool["remaining_weight"] = 687.0 + + await client.reset_spool_consumed_counter(42) + + mapped = _map_spoolman_spool(fake.spool) + assert fake.spool["extra"]["bambu_weight_used_baseline"] == json.dumps("313.0") + assert mapped["weight_used"] - mapped["weight_used_baseline"] == 0.0 + assert LABEL_WEIGHT - mapped["weight_used"] == 687.0, "still not full" + + +@pytest.mark.asyncio +async def test_the_baseline_is_stored_in_the_form_a_text_field_accepts(client, fake): + """The blocking defect in the first version of this fix. + + Spoolman registers an unseen extra key as ``text`` and then requires the + value to decode to a str, so ``json.dumps(263.0)`` -- the JSON number + ``263.0`` -- is rejected with "Value is not a string." and the PATCH 400s. + Pinning the string form is what makes the write land at all. + """ + await client.reset_spool_consumed_counter(42) + + stored = fake.spool["extra"]["bambu_weight_used_baseline"] + assert json.loads(stored) == "263.0", "stored as a JSON string, not a JSON number" + assert fake._validate_extra({"bambu_weight_used_baseline": stored}) is None + + +@pytest.mark.asyncio +async def test_the_baseline_key_is_registered_before_it_is_written(client, fake): + """It is not registered on an existing install, so the reset has to create it. + + The claim is the ordering, not the shape of the probe. Spoolman answers 400 + "Unknown extra field ." for a key it was never told about, so the + registration has to land before the PATCH that carries the value -- which is + what ``_ensure_extra_fields`` buys (#2903). How it decides the key is + missing is its own business: it probed the key directly when this test was + first written and lists the fields now, and pinning either spelling here + would fail on a change that does not touch the behaviour under test. + """ + assert "bambu_weight_used_baseline" not in fake.fields + + await client.reset_spool_consumed_counter(42) + + assert fake.fields["bambu_weight_used_baseline"] == "text" + registered = fake.log.index("POST /field/spool/bambu_weight_used_baseline") + written = fake.log.index("PATCH /spool/42") + assert registered < written, f"registration must precede the write: {fake.log}" + + +@pytest.mark.asyncio +async def test_a_baseline_written_as_text_reads_back_as_a_number(client, fake): + """The other half: the read side has to decode what the write side stores. + + ``_extract_extra_float`` used to require the decoded value to be a number, + so the string form it now has to write would have read back as None and the + baseline would have been silently ignored. + """ + await client.reset_spool_consumed_counter(42) + + mapped = _map_spoolman_spool(fake.spool) + + assert mapped["weight_used_baseline"] == 263.0 + assert mapped["weight_used"] - mapped["weight_used_baseline"] == 0.0 + + +def test_a_negative_stored_baseline_cannot_inflate_the_counter(): + """The write side clamps to >= 0 and the read side has to agree. + + It only shows on a spool whose remaining_weight and used_weight disagree -- + a hand-edited remaining, or a re-weigh -- because that is when the baseline + is a non-zero correction rather than a cancelling pair. Here Spoolman says + 263 g used while remaining says 300 g has gone; the baseline carries the + 37 g difference. A stored -100 drags the sum under zero, the outer max() + floors it at 0, and the 37 g correction is lost: consumed jumps to 300. + Clamping ``stored`` itself keeps the correction intact. + """ + spool = { + "id": 42, + "filament": {"id": 7, "name": "PLA Basic", "material": "PLA", "weight": LABEL_WEIGHT}, + "initial_weight": LABEL_WEIGHT, + "used_weight": 263.0, + "remaining_weight": 700.0, + "extra": {"bambu_weight_used_baseline": json.dumps("-100.0")}, + } + + mapped = _map_spoolman_spool(spool) + + assert mapped["weight_used"] == 300.0 + assert mapped["weight_used_baseline"] == 37.0, "the negative is discarded, not subtracted" + assert mapped["weight_used"] - mapped["weight_used_baseline"] == 263.0, "not inflated to 300" + + +def test_a_missing_baseline_and_a_zero_one_are_not_the_same_thing(): + """``or 0.0`` collapsed them, which is the distinction _extract_extra_float + exists to preserve. Both map to a zero baseline, but by different routes and + the helper has to keep answering None for the absent one.""" + from backend.app.api.routes._spoolman_helpers import ( + BAMBU_WEIGHT_USED_BASELINE_KEY, + _extract_extra_float, + ) + + assert _extract_extra_float({}, BAMBU_WEIGHT_USED_BASELINE_KEY) is None + assert ( + _extract_extra_float({BAMBU_WEIGHT_USED_BASELINE_KEY: json.dumps("0.0")}, BAMBU_WEIGHT_USED_BASELINE_KEY) == 0.0 + ) + # Both spellings have to read alike, so an install that already stored the + # number form before this fix keeps working. + assert ( + _extract_extra_float({BAMBU_WEIGHT_USED_BASELINE_KEY: json.dumps(263.0)}, BAMBU_WEIGHT_USED_BASELINE_KEY) + == 263.0 + ) + + +def test_the_writer_and_the_reader_name_the_same_key(): + """The key is declared twice on purpose (services/spoolman.py keeps the + client free of the routes package), so nothing but this assert holds the + two together. A divergence would not fail loudly anywhere else: the reset + returns 200, the value lands under the writer's key, and the reader never + finds it, so the counter simply never zeroes. And because Spoolman fixes an + extra field's type on its first write, a stray key cannot be tidied up + afterwards either.""" + from backend.app.api.routes import _spoolman_helpers + from backend.app.services import spoolman + + assert spoolman.BAMBU_WEIGHT_USED_BASELINE_KEY == _spoolman_helpers.BAMBU_WEIGHT_USED_BASELINE_KEY