From f46d115ce0bd11821c9db1c4b73818c48309f493 Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Mon, 12 Oct 2026 16:09:27 -0700 Subject: [PATCH 1/5] Refuse quantity-led serving names in update_custom_food too update_custom_food quietly split a leading number off serving_name ("2 cookies" became "cookies" x 2), while add_recipe/update_recipe refuse the same input. Guessing the server's name parser proved fragile (mixed numbers, "8 fl oz", unicode fractions), so both tools now fail fast through one shared _serving_name helper: the number goes in serving_quantity. Drops _QUANTITY_LED_NAME and its splitting branch. Resizing a custom food still resets the quantity to serving_quantity (or 1), as before. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 ++-- src/cronometer_api_mcp/client.py | 37 ++++++---------- src/cronometer_api_mcp/server.py | 2 +- tests/test_client.py | 21 +++++++++ tests/test_custom_food_edit.py | 74 +++++++++++++------------------- tests/test_recipe_edit.py | 27 +++--------- 6 files changed, 78 insertions(+), 91 deletions(-) diff --git a/README.md b/README.md index dbfaf87..ad0cbcc 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 `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 +recipe 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..09953bd 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(). @@ -876,9 +872,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 +889,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) @@ -2253,9 +2238,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 +2251,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 " diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index b2fa58a..7263706 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -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..3a6d433 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1181,3 +1181,24 @@ 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", + "↉", +] diff --git a/tests/test_custom_food_edit.py b/tests/test_custom_food_edit.py index fda421a..2b7aeac 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.""" - client, state = make_cold_client(tmp_path, [weight_default_food(), OK]) +def test_update_custom_food_keeps_inner_numbers(tmp_path, serving_name): + client, state = make_cold_client(tmp_path, [custom_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, 1) 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".""" diff --git a/tests/test_recipe_edit.py b/tests/test_recipe_edit.py index 32766ea..a709cfa 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,26 +1264,6 @@ 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", - "↉", -] - @pytest.mark.parametrize("serving_name", LEADING_QUANTITY_NAMES) def test_create_recipe_rejects_leading_quantity(tmp_path, serving_name): From c1d72a5fbc68f7356834dbe4874c43020a367e6f Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Mon, 12 Oct 2026 16:13:50 -0700 Subject: [PATCH 2/5] Apply the serving-name rule to add_custom_food too create_custom_food defaulted to "1 serving" and relied on the server to split the number off a new measure's name. It now takes serving_quantity (default 1, sent as the measure's amount), defaults the name to "serving", and runs both through the same checks as the edit tools before any request. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 +++--- src/cronometer_api_mcp/client.py | 11 ++++++--- src/cronometer_api_mcp/server.py | 8 ++++-- tests/test_client.py | 42 ++++++++++++++++++++++++++++++++ tests/test_custom_food_edit.py | 4 +-- 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index ad0cbcc..15781d3 100644 --- a/README.md +++ b/README.md @@ -224,10 +224,10 @@ every save, so the client sends none. No hard-delete endpoint for foods is known `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. Serving names are sent exactly as given with an explicit quantity, -so `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 -recipe 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. +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 recipe 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 09953bd..7ff3678 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -663,14 +663,17 @@ 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. + all nutrient data on a per-100g basis. The serving is + serving_quantity units of serving_name; a name that leads with a + quantity ("2 cookies") is refused. Args: extra_nutrients: Additional nutrients beyond the core macros above @@ -685,6 +688,8 @@ def create_custom_food( Returns {"food_id": int, "measure_id": int | None}. """ + serving_name = _serving_name(serving_name) + _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 @@ -739,7 +744,7 @@ def create_custom_food( "id": 0, "name": serving_name, "value": serving_grams, - "amount": 1.0, + "amount": serving_quantity, "type": "Atomic", } ], diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index 7263706..e7ed558 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,10 @@ 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_name: Unit name for the serving (default "serving"). A name + that starts with a number ("2 cookies") is refused. serving_grams: Weight of one serving in grams (default 100). + serving_quantity: Units per serving (default 1), e.g. 2 "cookies". """ try: client = _get_client() @@ -847,6 +850,7 @@ 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 diff --git a/tests/test_client.py b/tests/test_client.py index 3a6d433..bc7629f 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1202,3 +1202,45 @@ def fake_login() -> None: "⅟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_cold_client(tmp_path, [{"result": "SUCCESS", "id": 555}]) + + _create(client, serving_grams=30, **kwargs) + + (measure,) = state["payloads"][0]["data"]["measures"] + assert (measure["name"], measure["amount"], measure["value"]) == (name, amount, 30) + + +@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 ("", " ")] + + [ + ({"serving_quantity": q}, "serving_quantity") + for q 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"] == [] diff --git a/tests/test_custom_food_edit.py b/tests/test_custom_food_edit.py index 2b7aeac..ccb972d 100644 --- a/tests/test_custom_food_edit.py +++ b/tests/test_custom_food_edit.py @@ -280,12 +280,12 @@ def test_update_custom_food_rejects_blank_serving_name(tmp_path, serving_name): "serving_name", ["cookie (2 per pack)", "serving #2", "half (½) cup"] ) def test_update_custom_food_keeps_inner_numbers(tmp_path, serving_name): - client, state = make_cold_client(tmp_path, [custom_food(), OK]) + client, state = make_cold_client(tmp_path, [weight_default_food(), OK]) client.update_custom_food(FOOD_ID, serving_name=f" {serving_name} ") (measure,) = state["payloads"][1]["data"]["measures"] - assert (measure["name"], measure["amount"]) == (serving_name, 1) + assert (measure["name"], measure["amount"]) == (serving_name, 400) def test_update_custom_food_serving_name_alone_keeps_quantity(tmp_path): From 16843e2f0be25a99d59981e9ffb2b3c8127a58e3 Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Mon, 12 Oct 2026 16:15:21 -0700 Subject: [PATCH 3/5] Refuse non-positive serving_grams in create_custom_food A serving_grams of 0 or less used to fall back to a scale of 1, storing the per-serving numbers as per-100g. It is now checked with _check_positive before any request, like update_custom_food (NaN and inf are refused too). Co-Authored-By: Claude Opus 5.5 --- src/cronometer_api_mcp/client.py | 3 ++- tests/test_client.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index 7ff3678..049dacb 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -689,9 +689,10 @@ def create_custom_food( Returns {"food_id": int, "measure_id": int | None}. """ 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) diff --git a/tests/test_client.py b/tests/test_client.py index bc7629f..00a3e61 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1234,8 +1234,9 @@ def test_create_custom_food_serving_measure(tmp_path, kwargs, name, amount): ] + [({"serving_name": n}, "serving_name must not be empty") for n in ("", " ")] + [ - ({"serving_quantity": q}, "serving_quantity") - for q in (0, -1, float("nan"), float("inf")) + ({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): From 83108e61edaad86377b80e0580e8b73a38f7d4a4 Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Mon, 12 Oct 2026 16:23:26 -0700 Subject: [PATCH 4/5] Settle custom-food servings after create; fail fast on unusable servings - Split _settle_recipe_serving into a shared _settle_serving (read the serving back, re-send once if the server re-parsed its name, raise if it still doesn't match) plus the recipe-only nutrient-derivation check. create_custom_food now uses it, so "serving (1/2 batch)" no longer comes back as 0.5 x "batch)", and it returns the real measure_id (the add_custom_food tool no longer re-fetches the food for it). - update_custom_food raises instead of falling back to 100 g or a scale of 1 when the default serving's weight isn't a positive finite number, and refuses serving or nutrient edits on a food with no serving measure. - Share _is_positive_number between _check_positive and diary enrichment. - serving_grams docstrings: total weight of all serving_quantity units. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/cronometer_api_mcp/client.py | 135 +++++++++++++++++++++---------- src/cronometer_api_mcp/server.py | 8 +- tests/test_client.py | 73 ++++++++++++++++- tests/test_custom_food_edit.py | 42 ++++++++++ 5 files changed, 207 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 15781d3..569fd75 100644 --- a/README.md +++ b/README.md @@ -226,7 +226,7 @@ readable by id, and diary entries that use it are kept). 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 recipe measure's name (it does for some names when the quantity is 1, e.g. +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 049dacb..1019d3e 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -669,11 +669,12 @@ def create_custom_food( ) -> 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. The serving is - serving_quantity units of serving_name; a name that leads with a - quantity ("2 cookies") is refused. + 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 @@ -686,7 +687,7 @@ 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) @@ -762,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.get("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 @@ -910,20 +928,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, @@ -934,13 +938,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} @@ -1029,7 +1060,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 @@ -1173,8 +1204,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)"). @@ -1185,6 +1233,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), @@ -1192,20 +1241,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"], @@ -1215,12 +1264,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)" ) @@ -2027,11 +2076,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"), @@ -2271,8 +2316,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 e7ed558..909ac40 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -832,7 +832,8 @@ def add_custom_food( named macro args already cover. serving_name: Unit name for the serving (default "serving"). A name that starts with a number ("2 cookies") is refused. - serving_grams: Weight of one serving in grams (default 100). + 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: @@ -852,11 +853,6 @@ def add_custom_food( 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"], diff --git a/tests/test_client.py b/tests/test_client.py index 00a3e61..c1675af 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", @@ -1218,12 +1220,54 @@ def _create(client, **kwargs): ], ) def test_create_custom_food_serving_measure(tmp_path, kwargs, name, amount): - client, state = make_cold_client(tmp_path, [{"result": "SUCCESS", "id": 555}]) + client, state = make_server_client(tmp_path, FakeFoodServer()) - _create(client, serving_grams=30, **kwargs) + 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( @@ -1245,3 +1289,26 @@ def test_create_custom_food_rejects_bad_serving(tmp_path, kwargs, match): with pytest.raises(ValueError, match=match): _create(client, **kwargs) assert state["payloads"] == [] + + +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 ccb972d..4e9792f 100644 --- a/tests/test_custom_food_edit.py +++ b/tests/test_custom_food_edit.py @@ -528,3 +528,45 @@ 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 From a0fbe0ba72db5f4ccd4d05ced506ed2691679a26 Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Mon, 12 Oct 2026 16:28:41 -0700 Subject: [PATCH 5/5] Fail fast on missing or unmatched default measures - create_custom_food and create_recipe index defaultMeasureId directly. - _default_measure returns None only for a food with no measures, and raises when defaultMeasureId matches none, instead of quietly using the first measure. - Tests for create's failure paths after the food exists: a failing read-back, a failing re-send, and a read-back missing the default measure or its id all raise with the food's id. Co-Authored-By: Claude Opus 5.5 --- src/cronometer_api_mcp/client.py | 14 +++++--- tests/test_client.py | 60 ++++++++++++++++++++++++++++++++ tests/test_custom_food_edit.py | 8 +++++ tests/test_recipe_edit.py | 19 ++++++++++ 4 files changed, 97 insertions(+), 4 deletions(-) diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index 1019d3e..ee080a2 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -768,7 +768,7 @@ def create_custom_food( food = self.get_food(food_id) _, serving = self._settle_serving( food, - food.get("defaultMeasureId"), + food["defaultMeasureId"], serving_name, serving_quantity, serving_grams, @@ -858,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, @@ -1143,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 ) diff --git a/tests/test_client.py b/tests/test_client.py index c1675af..522a64f 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1291,6 +1291,66 @@ def test_create_custom_food_rejects_bad_serving(tmp_path, kwargs, match): 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) diff --git a/tests/test_custom_food_edit.py b/tests/test_custom_food_edit.py index 4e9792f..e4c521f 100644 --- a/tests/test_custom_food_edit.py +++ b/tests/test_custom_food_edit.py @@ -570,3 +570,11 @@ def test_update_custom_food_refuses_unusable_serving_weight(tmp_path, value): 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 a709cfa..e39d55f 100644 --- a/tests/test_recipe_edit.py +++ b/tests/test_recipe_edit.py @@ -1265,6 +1265,25 @@ def test_get_food_details_plain_food_has_no_recipe_fields(monkeypatch): # --------------------------------------------------------------------------- +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) def test_create_recipe_rejects_leading_quantity(tmp_path, serving_name): """The app would show "1 2 bowls"; the number belongs in serving_quantity."""