diff --git a/README.md b/README.md index dbfaf87..569fd75 100644 --- a/README.md +++ b/README.md @@ -223,9 +223,11 @@ recipe, the server derives the nutrients and the "full recipe" weight from the ` every save, so the client sends none. No hard-delete endpoint for foods is known, so `delete_custom_food` sets `retired` instead (the food leaves search after a few seconds but stays readable by id, and diary entries that use it are kept). -Retired foods can't be edited. Recipe serving names are sent exactly as given with an explicit -quantity; when the server re-parses a new measure's name (it does for some names when the quantity -is 1, e.g. "serving (1/2 batch)"), the client re-sends the measure verbatim, which sticks. +Retired foods can't be edited. Serving names are sent exactly as given with an explicit quantity, +so `add_custom_food`, `update_custom_food`, `add_recipe` and `update_recipe` refuse a name that +starts with a number ("2 cookies"): pass `serving_quantity=2, serving_name="cookies"`. When the +server re-parses a new measure's name (it does for some names when the quantity is 1, e.g. +"serving (1/2 batch)"), the client re-sends the measure verbatim, which sticks. Recipe import is the one asynchronous operation: `import_recipe` returns a job id, and `poll_async_result` is polled until the server reports 100% progress and attaches the parsed ingredients. diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index fdec705..ee080a2 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -17,7 +17,6 @@ import logging import math import os -import re import struct import threading import time @@ -91,9 +90,6 @@ _GRAMS_DECIMALS = 2 _GRAMS_TOL = 0.01 -# A serving name that leads with a plain number, e.g. "2 cookies". -_QUANTITY_LED_NAME = re.compile(r"^\s*(\d+(?:\.\d+)?)\s+(\S.*)$") - # Nutrient IDs create_custom_food() already writes via its named macro args # (including the derived/negative-ID duplicates it sends alongside them). # extra_nutrients must not reuse one of these -- see create_custom_food(). @@ -667,14 +663,18 @@ def create_custom_food( sodium_mg: float = 0, saturated_fat_g: float = 0, extra_nutrients: dict[int, float] | None = None, - serving_name: str = "1 serving", + serving_name: str = "serving", serving_grams: float = 100.0, + serving_quantity: float = 1, ) -> dict: """Create a custom food in Cronometer. - Nutrient amounts are per the full serving (serving_grams). - They are normalized to per-100g internally, since Cronometer stores - all nutrient data on a per-100g basis. + Nutrient amounts are per the full serving: serving_quantity units of + serving_name, with serving_grams the total weight of all of them. They + are normalized to per-100g, since Cronometer stores all nutrient data + that way. A serving_name that leads with a quantity ("2 cookies") is + refused. The saved serving is read back and re-sent once if the server + re-parsed its name (see _settle_serving). Args: extra_nutrients: Additional nutrients beyond the core macros above @@ -687,10 +687,13 @@ def create_custom_food( above; doing so raises ValueError rather than silently duplicating or shadowing an entry. - Returns {"food_id": int, "measure_id": int | None}. + Returns {"food_id": int, "measure_id": int}. """ + serving_name = _serving_name(serving_name) + _check_positive("serving_grams", serving_grams) + _check_positive("serving_quantity", serving_quantity) # Cronometer stores nutrients per 100g -- normalize from per-serving. - scale = 100.0 / serving_grams if serving_grams > 0 else 1.0 + scale = 100.0 / serving_grams net_carbs = max(0, carbs_g - fiber_g) @@ -743,7 +746,7 @@ def create_custom_food( "id": 0, "name": serving_name, "value": serving_grams, - "amount": 1.0, + "amount": serving_quantity, "type": "Atomic", } ], @@ -760,8 +763,25 @@ def create_custom_food( if not food_id: raise CronometerError(f"Failed to create custom food: {data}") + try: + with self._food_lock(food_id): + food = self.get_food(food_id) + _, serving = self._settle_serving( + food, + food["defaultMeasureId"], + serving_name, + serving_quantity, + serving_grams, + recipe=False, + ) + except Exception as e: + # As in create_recipe: the food exists, so the id must reach the + # caller to prevent a duplicate. + raise CronometerError( + f"The custom food exists (id {food_id}); don't re-create it. {e}" + ) from e logger.info("Created custom food %r (id=%d)", name, food_id) - return {"food_id": food_id, "measure_id": None} + return {"food_id": food_id, "measure_id": serving["id"]} # ------------------------------------------------------------------ # Custom food edit / retire @@ -838,11 +858,17 @@ def _rename(food: dict, name: str | None) -> None: @staticmethod def _default_measure(food: dict) -> dict | None: + """The food's default measure, or None if it has no measures.""" measures = food.get("measures") or [] + if not measures: + return None for m in measures: - if m.get("id") == food.get("defaultMeasureId"): + if m["id"] == food["defaultMeasureId"]: return m - return measures[0] if measures else None + raise CronometerError( + f"Food {food['id']}'s default measure {food['defaultMeasureId']} " + f"isn't one of its measures; fix it in the Cronometer app" + ) def update_custom_food( self, @@ -876,9 +902,9 @@ def update_custom_food( weight therefore also resets the quantity (to serving_quantity, or 1) and makes the measure a named ("Atomic") serving. Otherwise a food first saved as "400 g" keeps amount=400 and type "Weight" and shows up - as "400 " with no weight of its own. A quantity leading - serving_name ("2 cookies") is moved into the quantity for the same - reason. A quantity set without serving_grams keeps the total weight, + as "400 " with no weight of its own. serving_name must + not lead with a quantity ("2 cookies"); pass it as serving_quantity. + A quantity set without serving_grams keeps the total weight, so the per-unit weight changes. (update_recipe keeps the quantity on a resize instead: its servings never have this "400 g" shape.) @@ -893,22 +919,11 @@ def update_custom_food( f"those instead." ) - # The server splits a leading quantity off a new measure's name - # ("10 pieces" is stored as amount=10, name="pieces") but not when - # an existing measure is re-sent, so do the same here. Done before - # validation so an inferred quantity is checked too. - quantity_label = "serving_quantity" if serving_name is not None: - led = _QUANTITY_LED_NAME.match(serving_name) - if led and float(led.group(1)) > 0: - serving_name = led.group(2) - if serving_quantity is None: - serving_quantity = float(led.group(1)) - quantity_label = "serving_name quantity" - + serving_name = _serving_name(serving_name) for label, v in ( ("serving_grams", serving_grams), - (quantity_label, serving_quantity), + ("serving_quantity", serving_quantity), ): if v is not None: _check_positive(label, v) @@ -919,20 +934,6 @@ def update_custom_food( self._rename(food, name) - measure = self._default_measure(food) - if measure is not None: - if serving_name is not None: - measure["name"] = serving_name - if serving_grams is not None: - measure["value"] = serving_grams - if serving_grams is not None or serving_quantity is not None: - measure["amount"] = ( - serving_quantity if serving_quantity is not None else 1.0 - ) - measure["type"] = "Atomic" - grams = (measure or {}).get("value") or 100.0 - scale = 100.0 / grams if grams > 0 else 1.0 - per_serving = { NUTRIENT_IDS["energy"]: calories, NUTRIENT_IDS["protein"]: protein_g, @@ -943,13 +944,40 @@ def update_custom_food( NUTRIENT_IDS["sodium"]: sodium_mg, NUTRIENT_IDS["saturated_fat"]: saturated_fat_g, } - updates = { - nid: round(v * scale, 2) - for nid, v in per_serving.items() - if v is not None - } - for nid, v in (extra_nutrients or {}).items(): - updates[nid] = round(v * scale, 2) + per_serving = {nid: v for nid, v in per_serving.items() if v is not None} + per_serving.update(extra_nutrients or {}) + + serving_args = (serving_name, serving_grams, serving_quantity) + measure = self._default_measure(food) + if measure is None and ( + per_serving or any(v is not None for v in serving_args) + ): + raise CronometerError( + f"Custom food {food_id} has no serving measure, so its " + f"serving and per-serving nutrients can't be set; fix it in " + f"the Cronometer app" + ) + if serving_name is not None: + measure["name"] = serving_name + if serving_grams is not None: + measure["value"] = serving_grams + if serving_grams is not None or serving_quantity is not None: + measure["amount"] = ( + serving_quantity if serving_quantity is not None else 1.0 + ) + measure["type"] = "Atomic" + updates = {} + if per_serving: + grams = measure["value"] + if not _is_positive_number(grams): + raise CronometerError( + f"Custom food {food_id}'s serving weighs {grams!r} g, so " + f"per-serving nutrients can't be converted; pass " + f"serving_grams too" + ) + updates = { + nid: round(v * 100.0 / grams, 2) for nid, v in per_serving.items() + } nutrients = food.setdefault("nutrients", []) by_id = {n["id"]: n for n in nutrients} @@ -1038,7 +1066,7 @@ def create_recipe( `grams` is always what the nutrient math uses. When omitted, the ingredient's own gram measure is resolved automatically. serving_name: Name of the default serving measure, stored exactly - as given (see _settle_recipe_serving). + as given (see _settle_serving). serving_grams: Grams in one serving. Defaults to the full batch weight (i.e. one serving = the whole recipe). servings_per_batch: Instead of serving_grams, size the serving as @@ -1121,7 +1149,7 @@ def create_recipe( try: with self._food_lock(food_id): food = self.get_food(food_id) - measure_id = food.get("defaultMeasureId") + measure_id = food["defaultMeasureId"] food, serving = self._settle_recipe_serving( food, measure_id, serving_name, serving_quantity, serving_grams ) @@ -1182,8 +1210,25 @@ def _recipe_serving(cls, food: dict) -> dict: def _settle_recipe_serving( self, food: dict, measure_id: int, name: str, quantity: float, grams: float ) -> tuple[dict, dict]: - """Make sure the saved serving reads back as (name, quantity, grams), - and that the server derived the recipe's nutrients. + """_settle_serving, then check that the server derived the recipe's + nutrients (a re-send goes out with them empty, so this proves it).""" + food, serving = self._settle_serving( + food, measure_id, name, quantity, grams, recipe=True + ) + _check_derived_nutrients(food) + return food, serving + + def _settle_serving( + self, + food: dict, + measure_id: int, + name: str, + quantity: float, + grams: float, + *, + recipe: bool, + ) -> tuple[dict, dict]: + """Make sure the saved serving reads back as (name, quantity, grams). When the server creates a measure whose amount is 1 it may re-parse the name ("serving (1/2 batch)" was stored as amount 0.5, name "batch)"). @@ -1194,6 +1239,7 @@ def _settle_recipe_serving( Returns the read-back (food, serving). """ food_id = food["id"] + kind = "Recipe" if recipe else "Custom food" for attempt in range(2): serving = next( (m for m in food.get("measures") or [] if m.get("id") == measure_id), @@ -1201,20 +1247,20 @@ def _settle_recipe_serving( ) if serving is None: raise CronometerError( - f"Recipe {food_id} has no measure {measure_id} after saving" + f"{kind} {food_id} has no measure {measure_id} after saving" ) if ( serving["name"] == name and math.isclose(serving["amount"], quantity) and math.isclose(serving["value"], grams, rel_tol=0, abs_tol=_GRAMS_TOL) ): - _check_derived_nutrients(food) return food, serving if attempt: break logger.info( - "Server stored recipe %d's serving as %r x%s (%s g); re-sending " + "Server stored %s %d's serving as %r x%s (%s g); re-sending " "%r x%s (%s g)", + kind, food_id, serving["name"], serving["amount"], @@ -1224,12 +1270,12 @@ def _settle_recipe_serving( grams, ) serving.update(name=name, amount=quantity, value=grams) - # Empty again, so this read-back also proves the server derived them. - food["nutrients"] = [] + if recipe: + food["nutrients"] = [] self._save_food(food) food = self.get_food(food_id) raise CronometerError( - f"Recipe {food_id} was saved, but its serving reads back as " + f"{kind} {food_id} was saved, but its serving reads back as " f"{serving['name']!r} x{serving['amount']} ({serving['value']} g) " f"instead of {name!r} x{quantity} ({grams} g)" ) @@ -2036,11 +2082,7 @@ def enrich_diary_servings(self, diary: dict) -> dict: if measure: portion_value = measure.get("value") quantity = measure.get("amount", 1) - if not ( - isinstance(quantity, (int, float)) - and math.isfinite(quantity) - and quantity > 0 - ): + if not _is_positive_number(quantity): quantity = None entry["serving_size"] = { "measure_id": measure.get("id"), @@ -2253,9 +2295,9 @@ def _is_fixed_recipe_measure(name: str) -> bool: return _measure_key(name) in _RECIPE_FIXED_MEASURES -def _recipe_serving_name(name: str) -> str: - """Strip a recipe serving name; refuse blanks, built-in measure names, and - names that start with a quantity (the app would show "1 2 bowls").""" +def _serving_name(name: str) -> str: + """Strip a serving name; refuse blanks and names that start with a + quantity (the app would show "1 2 bowls").""" name = name.strip() if not name: raise ValueError("serving_name must not be empty") @@ -2266,6 +2308,12 @@ def _recipe_serving_name(name: str) -> str: f"serving_quantity and only the unit as serving_name (e.g. " f"serving_quantity=2, serving_name='bowls')" ) + return name + + +def _recipe_serving_name(name: str) -> str: + """_serving_name, also refusing the recipe's built-in measure names.""" + name = _serving_name(name) if _is_fixed_recipe_measure(name): raise ValueError( f"serving_name {name!r} is reserved for one of the recipe's " @@ -2274,8 +2322,12 @@ def _recipe_serving_name(name: str) -> str: return name +def _is_positive_number(v: object) -> bool: + return isinstance(v, (int, float)) and math.isfinite(v) and v > 0 + + def _check_positive(label: str, v: float) -> None: - if not (math.isfinite(v) and v > 0): + if not _is_positive_number(v): raise ValueError(f"{label} must be a positive number, got {v!r}") diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index b2fa58a..909ac40 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -805,8 +805,9 @@ def add_custom_food( sodium_mg: float = 0, saturated_fat_g: float = 0, extra_nutrients: dict[int, float] | None = None, - serving_name: str = "1 serving", + serving_name: str = "serving", serving_grams: float = 100.0, + serving_quantity: float = 1, ) -> str: """Create a custom food in Cronometer with specified nutrition. @@ -829,8 +830,11 @@ def add_custom_food( its name) and valued per the full serving. IDs aren't validated, so a wrong one writes the wrong nutrient; must not reuse an ID the named macro args already cover. - serving_name: Name for the serving size (default "1 serving"). - serving_grams: Weight of one serving in grams (default 100). + serving_name: Unit name for the serving (default "serving"). A name + that starts with a number ("2 cookies") is refused. + serving_grams: Total weight of the serving (all serving_quantity + units) in grams (default 100). + serving_quantity: Units per serving (default 1), e.g. 2 "cookies". """ try: client = _get_client() @@ -847,12 +851,8 @@ def add_custom_food( extra_nutrients=extra_nutrients, serving_name=serving_name, serving_grams=serving_grams, + serving_quantity=serving_quantity, ) - - # Fetch back to get the server-assigned measure_id - food_data = client.get_food(result["food_id"]) - result["measure_id"] = food_data.get("defaultMeasureId") - return _ok( { "food_id": result["food_id"], @@ -911,7 +911,7 @@ def update_custom_food( an ID the named args already cover. serving_name: New name for the default serving. The app shows the serving as " "; a leading number ("2 cookies") - is taken as the quantity (see serving_quantity). + is refused: pass it as serving_quantity. serving_grams: New total weight of the default serving in grams. Also resets the quantity to serving_quantity (or 1). serving_quantity: How many units make up the default serving diff --git a/tests/test_client.py b/tests/test_client.py index fa58c33..522a64f 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -9,6 +9,7 @@ from __future__ import annotations import copy +import json import re import threading import time @@ -16,6 +17,7 @@ import pytest +from cronometer_api_mcp import server as mcp_server from cronometer_api_mcp.client import CronometerClient, CronometerError # Real response body observed from an expired session. @@ -1005,7 +1007,7 @@ def test_import_recipe_rejects_blank_input(tmp_path, monkeypatch): def test_create_custom_food_extra_nutrients_payload(tmp_path): """extra_nutrients are appended, normalized to per-100g like the macros.""" - client, state = make_cold_client(tmp_path, [{"result": "SUCCESS", "id": 555}]) + client, state = make_server_client(tmp_path, FakeFoodServer()) client.create_custom_food( "Test Food", @@ -1181,3 +1183,192 @@ def fake_login() -> None: assert state["login"] == 1 assert client._token == "FRESH_TOKEN" + + +LEADING_QUANTITY_NAMES = [ + "2 bowls", + "1.5 cups", + "1/2 cup", + "1 1/2 cups", + " 3 plates", + "½ cup", + "1½ cups", + ".5 cup", + "2x bowls", + "2-3 bowls", + "1,5 cups", + "2", + "⅓ batch", + "⅞ cup", + "¼", + "⅟2 cup", + "↉", +] + + +def _create(client, **kwargs): + return client.create_custom_food( + "Test Food", calories=200, protein_g=10, fat_g=5, carbs_g=20, **kwargs + ) + + +@pytest.mark.parametrize( + ("kwargs", "name", "amount"), + [ + ({}, "serving", 1), + ({"serving_name": " cookies ", "serving_quantity": 2}, "cookies", 2), + ], +) +def test_create_custom_food_serving_measure(tmp_path, kwargs, name, amount): + client, state = make_server_client(tmp_path, FakeFoodServer()) + + result = _create(client, serving_grams=30, **kwargs) + + assert state["endpoints"] == ["add_food", "get_food"] + (measure,) = state["payloads"][0]["data"]["measures"] + assert (measure["name"], measure["amount"], measure["value"]) == (name, amount, 30) + assert result == {"food_id": 4242, "measure_id": 9000} + + +@pytest.mark.parametrize( + "serving_name", ["serving (1/2 batch)", "cup 1/3 full", "slice (1/4 pie)"] +) +def test_create_custom_food_resends_reparsed_serving(tmp_path, serving_name): + """A new measure's name can be re-parsed by the server (server_reparse); + the serving is re-sent once, verbatim, and the nutrients are kept.""" + server = FakeFoodServer() + client, state = make_server_client(tmp_path, server) + + result = _create(client, serving_name=serving_name, serving_grams=250) + + assert state["endpoints"] == ["add_food", "get_food", "add_food", "get_food"] + sent = state["payloads"][2]["data"] + assert sent["nutrients"] == state["payloads"][0]["data"]["nutrients"] + (measure,) = server.foods[result["food_id"]]["measures"] + assert (measure["name"], measure["amount"], measure["value"]) == ( + serving_name, + 1, + 250, + ) + + +def test_create_custom_food_raises_with_id_when_serving_wont_settle(tmp_path): + """A serving still rewritten after the re-send is an error naming the + food id, since the food already exists.""" + server = FakeFoodServer() + client, state = make_server_client(tmp_path, server) + save = server._save + + def reparse_every_save(food): + food["measures"][0].update(name="batch)", amount=0.5) + return save(food) + + server._save = reparse_every_save + + with pytest.raises(CronometerError, match=r"exists \(id 4242\).*reads back as"): + _create(client, serving_name="serving (1/2 batch)") + assert state["endpoints"] == ["add_food", "get_food", "add_food", "get_food"] + + +@pytest.mark.parametrize( + ("kwargs", "match"), + [ + ({"serving_name": n}, "pass the number as serving_quantity") + for n in LEADING_QUANTITY_NAMES + ] + + [({"serving_name": n}, "serving_name must not be empty") for n in ("", " ")] + + [ + ({field: v}, field) + for field in ("serving_grams", "serving_quantity") + for v in (0, -1, float("nan"), float("inf")) + ], +) +def test_create_custom_food_rejects_bad_serving(tmp_path, kwargs, match): + client, state = make_cold_client(tmp_path, []) + + with pytest.raises(ValueError, match=match): + _create(client, **kwargs) + assert state["payloads"] == [] + + +def _failing_server(fail_on: str, call: int) -> FakeFoodServer: + """A FakeFoodServer whose `call`-th request to `fail_on` raises.""" + fake = FakeFoodServer() + handle, seen = fake.handle, {"n": 0} + + def flaky(endpoint, body): + if endpoint == f"/api/v2/{fail_on}": + seen["n"] += 1 + if seen["n"] == call: + raise CronometerError(f"{fail_on} timed out") + return handle(endpoint, body) + + fake.handle = flaky + return fake + + +@pytest.mark.parametrize( + ("fail_on", "call", "serving_name", "add_foods"), + [ + ("get_food", 1, "serving", 1), + ("add_food", 2, "serving (1/2 batch)", 2), + ], +) +def test_create_custom_food_failure_after_save_names_the_id( + tmp_path, fail_on, call, serving_name, add_foods +): + client, state = make_server_client(tmp_path, _failing_server(fail_on, call)) + + with pytest.raises(CronometerError, match=r"exists \(id 4242\)"): + _create(client, serving_name=serving_name) + assert state["endpoints"].count("add_food") == add_foods + + +@pytest.mark.parametrize( + ("break_read_back", "match"), + [ + (lambda food: food.update(measures=[]), "has no measure 9000"), + (lambda food: food.pop("defaultMeasureId"), "'defaultMeasureId'"), + ], +) +def test_create_custom_food_read_back_without_default_measure( + tmp_path, break_read_back, match +): + fake = FakeFoodServer() + client, state = make_server_client(tmp_path, fake) + handle = fake.handle + + def broken(endpoint, body): + out = handle(endpoint, body) + if endpoint == "/api/v2/get_food": + break_read_back(out) + return out + + fake.handle = broken + + with pytest.raises(CronometerError, match=rf"exists \(id 4242\).*{match}"): + _create(client) + assert state["endpoints"] == ["add_food", "get_food"] + + +def test_add_custom_food_tool_sends_serving_quantity(tmp_path, monkeypatch): + client, state = make_server_client(tmp_path, FakeFoodServer()) + monkeypatch.setattr(mcp_server, "_get_client", lambda: client) + + out = json.loads( + mcp_server.add_custom_food( + "Cookies", + calories=100, + protein_g=2, + fat_g=5, + carbs_g=12, + serving_name="cookies", + serving_grams=30, + serving_quantity=2, + ) + ) + + assert out["status"] == "success" + assert (out["food_id"], out["measure_id"]) == (4242, 9000) + (measure,) = state["payloads"][0]["data"]["measures"] + assert (measure["name"], measure["amount"], measure["value"]) == ("cookies", 2, 30) diff --git a/tests/test_custom_food_edit.py b/tests/test_custom_food_edit.py index fda421a..e4c521f 100644 --- a/tests/test_custom_food_edit.py +++ b/tests/test_custom_food_edit.py @@ -11,7 +11,7 @@ import threading import pytest -from test_client import make_cold_client +from test_client import LEADING_QUANTITY_NAMES, make_cold_client from cronometer_api_mcp.client import CronometerError @@ -156,13 +156,13 @@ def test_update_custom_food_serving_overrides_measure_and_scale(tmp_path): client, state = make_cold_client(tmp_path, [custom_food(), OK]) client.update_custom_food( - FOOD_ID, serving_name="1 cup", serving_grams=200, calories=400 + FOOD_ID, serving_name="cup", serving_grams=200, calories=400 ) data = state["payloads"][1]["data"] (measure,) = data["measures"] assert measure["id"] == 900 - assert measure["name"] == "cup" # the leading 1 is the quantity + assert measure["name"] == "cup" assert measure["amount"] == 1.0 assert measure["value"] == 200 assert nutrients_by_id(state["payloads"][1])[208] == 200.0 # 400 per 200 g @@ -207,13 +207,6 @@ def test_update_custom_food_serving_quantity(tmp_path): assert (measure["amount"], measure["value"]) == (2, 30) -@pytest.mark.parametrize( - "kwargs", - [ - {"serving_name": "2 cookies"}, - {"serving_name": "cookies", "serving_quantity": 2}, - ], -) @pytest.mark.parametrize( ("measure_id", "quantity", "unit", "grams", "counts"), [ @@ -222,14 +215,16 @@ def test_update_custom_food_serving_quantity(tmp_path): ], ) def test_update_custom_food_serving_size_in_diary( - tmp_path, monkeypatch, kwargs, measure_id, quantity, unit, grams, counts + tmp_path, monkeypatch, measure_id, quantity, unit, grams, counts ): food = custom_food() food["measures"].append( {"id": 901, "name": "cookie", "amount": 1, "value": 15, "type": "Atomic"} ) client, state = make_cold_client(tmp_path, [food, OK]) - client.update_custom_food(FOOD_ID, serving_grams=30, **kwargs) + client.update_custom_food( + FOOD_ID, serving_grams=30, serving_name="cookies", serving_quantity=2 + ) saved = state["payloads"][1]["data"] monkeypatch.setattr(client, "get_foods", lambda ids: [saved]) @@ -262,32 +257,35 @@ def test_update_custom_food_serving_size_in_diary( assert energy["amount"] == calories +@pytest.mark.parametrize("kwargs", [{}, {"serving_quantity": 2}]) +@pytest.mark.parametrize("serving_name", LEADING_QUANTITY_NAMES) +def test_update_custom_food_rejects_leading_quantity(tmp_path, serving_name, kwargs): + client, state = make_cold_client(tmp_path, []) + + with pytest.raises(ValueError, match="pass the number as serving_quantity"): + client.update_custom_food(FOOD_ID, serving_name=serving_name, **kwargs) + assert state["payloads"] == [] + + +@pytest.mark.parametrize("serving_name", ["", " "]) +def test_update_custom_food_rejects_blank_serving_name(tmp_path, serving_name): + client, state = make_cold_client(tmp_path, []) + + with pytest.raises(ValueError, match="serving_name must not be empty"): + client.update_custom_food(FOOD_ID, serving_name=serving_name) + assert state["payloads"] == [] + + @pytest.mark.parametrize( - ("given", "kwargs", "name", "amount"), - [ - ("1 serving", {}, "serving", 1.0), - ("10 pieces", {}, "pieces", 10.0), - ("1.5 cups", {}, "cups", 1.5), - ("2 cookies", {"serving_quantity": 3}, "cookies", 3), - ("1/2 cup", {"serving_grams": 120}, "1/2 cup", 1.0), - ("0 calorie syrup", {"serving_grams": 15}, "0 calorie syrup", 1.0), - ], + "serving_name", ["cookie (2 per pack)", "serving #2", "half (½) cup"] ) -def test_update_custom_food_quantity_led_serving_name( - tmp_path, given, kwargs, name, amount -): - """A plain number leading serving_name becomes the quantity, as the server - does for a new measure; an explicit serving_quantity wins.""" +def test_update_custom_food_keeps_inner_numbers(tmp_path, serving_name): client, state = make_cold_client(tmp_path, [weight_default_food(), OK]) - client.update_custom_food(FOOD_ID, serving_name=given, **kwargs) + client.update_custom_food(FOOD_ID, serving_name=f" {serving_name} ") (measure,) = state["payloads"][1]["data"]["measures"] - assert (measure["name"], measure["amount"], measure["type"]) == ( - name, - amount, - "Atomic", - ) + assert (measure["name"], measure["amount"]) == (serving_name, 400) def test_update_custom_food_serving_name_alone_keeps_quantity(tmp_path): @@ -313,16 +311,6 @@ def test_update_custom_food_rejects_bad_serving_numbers(tmp_path, field, bad): assert state["payloads"] == [] -def test_update_custom_food_rejects_overflowing_name_quantity(tmp_path): - """A leading number too large for a float would become inf; it is - rejected before anything is fetched or saved.""" - client, state = make_cold_client(tmp_path, []) - - with pytest.raises(ValueError, match="serving_name quantity"): - client.update_custom_food(FOOD_ID, serving_name="9" * 400 + " cookies") - assert state["payloads"] == [] - - def test_update_custom_food_quantity_alone_keeps_total_weight(tmp_path): """Without serving_grams, a new quantity keeps the serving's total weight: "2 cookies = 30 g" becomes "3 cookies = 30 g".""" @@ -540,3 +528,53 @@ def stalling_post(endpoint, json=None): ] assert state["payloads"][1]["data"]["name"] == "A" assert state["payloads"][3]["data"]["retired"] is True + + +@pytest.mark.parametrize( + "kwargs", + [ + {"serving_name": "cup"}, + {"serving_grams": 30}, + {"serving_quantity": 2}, + {"calories": 100}, + {"extra_nutrients": {601: 5}}, + ], +) +def test_update_custom_food_without_measure_refuses_serving_edits(tmp_path, kwargs): + client, state = make_cold_client(tmp_path, [custom_food(measures=[])]) + + with pytest.raises(CronometerError, match="has no serving measure"): + client.update_custom_food(FOOD_ID, **kwargs) + assert len(state["payloads"]) == 1 # the read, no save + + +def test_update_custom_food_without_measure_still_renames(tmp_path): + client, state = make_cold_client(tmp_path, [custom_food(measures=[]), OK]) + + client.update_custom_food(FOOD_ID, name="Renamed") + + assert state["payloads"][1]["data"]["name"] == "Renamed" + + +@pytest.mark.parametrize("value", [0, -5, None, float("nan"), float("inf"), "50"]) +def test_update_custom_food_refuses_unusable_serving_weight(tmp_path, value): + """Per-serving nutrients need a real serving weight to convert to per-100g; + there is no fallback scale.""" + food = custom_food() + food["measures"][0]["value"] = value + client, state = make_cold_client(tmp_path, [food, food, OK]) + + with pytest.raises(CronometerError, match="per-serving nutrients"): + client.update_custom_food(FOOD_ID, calories=100) + assert len(state["payloads"]) == 1 + + client.update_custom_food(FOOD_ID, calories=100, serving_grams=50) + assert nutrients_by_id(state["payloads"][2])[208] == 200.0 + + +def test_update_custom_food_refuses_unmatched_default_measure(tmp_path): + client, state = make_cold_client(tmp_path, [custom_food(defaultMeasureId=901)]) + + with pytest.raises(CronometerError, match="default measure 901 isn't one"): + client.update_custom_food(FOOD_ID, calories=100) + assert len(state["payloads"]) == 1 # the read, no save diff --git a/tests/test_recipe_edit.py b/tests/test_recipe_edit.py index 32766ea..e39d55f 100644 --- a/tests/test_recipe_edit.py +++ b/tests/test_recipe_edit.py @@ -11,7 +11,12 @@ import httpx import pytest -from test_client import FakeFoodServer, make_server_client, server_reparse +from test_client import ( + LEADING_QUANTITY_NAMES, + FakeFoodServer, + make_server_client, + server_reparse, +) from cronometer_api_mcp import server from cronometer_api_mcp.client import CronometerError @@ -1259,25 +1264,24 @@ def test_get_food_details_plain_food_has_no_recipe_fields(monkeypatch): # Leading quantities, derived nutrients, serving weight # --------------------------------------------------------------------------- -LEADING_QUANTITY_NAMES = [ - "2 bowls", - "1.5 cups", - "1/2 cup", - "1 1/2 cups", - " 3 plates", - "½ cup", - "1½ cups", - ".5 cup", - "2x bowls", - "2-3 bowls", - "1,5 cups", - "2", - "⅓ batch", - "⅞ cup", - "¼", - "⅟2 cup", - "↉", -] + +def test_create_recipe_read_back_without_default_measure_id(tmp_path): + fake = FakeFoodServer(ingredient_foods=[EGG]) + client, _ = make_server_client(tmp_path, fake) + handle = fake.handle + + def broken(endpoint, body): + out = handle(endpoint, body) + if endpoint == "/api/v2/get_food": + out.pop("defaultMeasureId") + return out + + fake.handle = broken + + with pytest.raises( + CronometerError, match=r"exists \(id 4242\).*'defaultMeasureId'" + ): + client.create_recipe("R", ingredients=[(1234, 100)]) @pytest.mark.parametrize("serving_name", LEADING_QUANTITY_NAMES)