From c91a3c159e7862d432cbf032eeaae1f428cdeb7d Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Tue, 13 Oct 2026 09:51:24 -0700 Subject: [PATCH 1/2] Handle defaultMeasureId 0 when creating and editing custom foods Cronometer stores a custom food whose only measure is "g" (1 g) with defaultMeasureId 0, so add_custom_food(serving_name="g", serving_grams=1) failed after creating the food, and update_custom_food refused it. - create_custom_food takes the food's one measure instead of the default. - _default_measure treats id 0 as "no explicit default": the sole measure is the default; several measures are refused as ambiguous. - create_recipe identifies its serving as the one non-fixed measure, as update_recipe already does, instead of trusting defaultMeasureId. Co-Authored-By: Claude Opus 5.5 --- src/cronometer_api_mcp/client.py | 28 ++++++++--- tests/test_client.py | 80 ++++++++++++++++++++++++++++++-- tests/test_recipe_edit.py | 39 +++++++++++++--- 3 files changed, 130 insertions(+), 17 deletions(-) diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index ee080a2..ec3b0fd 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["defaultMeasureId"], + _only_measure(food)["id"], serving_name, serving_quantity, serving_grams, @@ -858,10 +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.""" + """The food's default measure, or None if it has no measures. + + defaultMeasureId 0 means the server stored no explicit default (seen + live for a food whose only measure is "g"); the sole measure is then + the default, and with several it's ambiguous, so that's refused. + """ measures = food.get("measures") or [] if not measures: return None + if food["defaultMeasureId"] == 0: + return _only_measure(food) for m in measures: if m["id"] == food["defaultMeasureId"]: return m @@ -1091,8 +1098,7 @@ def create_recipe( _check_serving_fits(serving_grams, batch_grams) ingredient_rows = self._ingredient_rows(parsed) - # The first measure in the array becomes the server-assigned - # defaultMeasureId, so the serving measure leads. + # The serving leads so the server makes it the app's default measure. measures = [ { "id": 0, @@ -1145,11 +1151,10 @@ def create_recipe( if not food_id: raise CronometerError(f"Failed to create recipe: {data}") - # The first measure becomes the default, which identifies the serving. try: with self._food_lock(food_id): food = self.get_food(food_id) - measure_id = food["defaultMeasureId"] + measure_id = self._recipe_serving(food)["id"] food, serving = self._settle_recipe_serving( food, measure_id, serving_name, serving_quantity, serving_grams ) @@ -2414,6 +2419,17 @@ def _serving_fraction(batch_grams: float, servings_per_batch: float) -> float: return round(batch_grams / servings_per_batch, _GRAMS_DECIMALS) +def _only_measure(food: dict) -> dict: + """The food's one measure, refusing a food with none or several.""" + measures = food.get("measures") or [] + if len(measures) != 1: + raise CronometerError( + f"Food {food['id']} has {len(measures)} measures " + f"{[m.get('name') for m in measures]}, expected exactly one" + ) + return measures[0] + + def _gram_measure_id(food: dict) -> int: """Return the food's gram measure ID, for use as a recipe ingredient. diff --git a/tests/test_client.py b/tests/test_client.py index 522a64f..c3a581f 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -186,7 +186,8 @@ class FakeFoodServer: """A stateful stand-in for add_food / get_food / get_foods. add_food upserts like the real endpoint: new foods and measures get ids, - the first measure becomes the default, new measure names are re-parsed + the first measure becomes the default (unless it's a lone "g", which + keeps defaultMeasureId 0, as seen live), new measure names are re-parsed (server_reparse), and a recipe's "full recipe" weight and per-100g nutrients are derived from its ingredients (derive_nutrients=False keeps whatever nutrients were sent, simulating a save that derived nothing). @@ -245,7 +246,7 @@ def _save(self, food: dict) -> dict: m["id"] = self.next_measure_id self.next_measure_id += 1 m["name"], m["amount"] = server_reparse(m["name"], m["amount"]) - if not food.get("defaultMeasureId"): + if not food.get("defaultMeasureId") and not _only_gram(food): food["defaultMeasureId"] = food["measures"][0]["id"] if food.get("ingredients"): batch = sum(i["grams"] for i in food["ingredients"]) @@ -267,6 +268,10 @@ def _save(self, food: dict) -> dict: return {"result": "SUCCESS", "messages": [], "id": food["id"]} +def _only_gram(food: dict) -> bool: + return [(m["name"], m["value"]) for m in food["measures"]] == [("g", 1)] + + def make_server_client(tmp_path: Path, server: FakeFoodServer): """A logged-in client backed by `server`; state records every request.""" client = CronometerClient(session_path=tmp_path / "session.json") @@ -1327,11 +1332,14 @@ def test_create_custom_food_failure_after_save_names_the_id( @pytest.mark.parametrize( ("break_read_back", "match"), [ - (lambda food: food.update(measures=[]), "has no measure 9000"), - (lambda food: food.pop("defaultMeasureId"), "'defaultMeasureId'"), + (lambda food: food.update(measures=[]), r"has 0 measures \[\]"), + ( + lambda food: food["measures"].append({**food["measures"][0], "id": 1}), + r"has 2 measures \['serving', 'serving'\]", + ), ], ) -def test_create_custom_food_read_back_without_default_measure( +def test_create_custom_food_read_back_without_one_measure( tmp_path, break_read_back, match ): fake = FakeFoodServer() @@ -1351,6 +1359,68 @@ def broken(endpoint, body): assert state["endpoints"] == ["add_food", "get_food"] +@pytest.mark.parametrize("default", [0, 9000, None]) +def test_create_custom_food_ignores_default_measure_id(tmp_path, default): + """The one measure sent is the serving, whatever defaultMeasureId says.""" + fake = FakeFoodServer() + client, state = make_server_client(tmp_path, fake) + handle = fake.handle + + def read_back(endpoint, body): + out = handle(endpoint, body) + if endpoint == "/api/v2/get_food": + out["defaultMeasureId"] = default + return out + + fake.handle = read_back + + assert _create(client)["measure_id"] == 9000 + assert state["endpoints"] == ["add_food", "get_food"] + + +def _create_gram_food(tmp_path): + fake = FakeFoodServer() + client, state = make_server_client(tmp_path, fake) + result = _create(client, serving_name="g", serving_grams=1) + return client, state, fake, result + + +def test_create_gram_custom_food_with_zero_default_measure(tmp_path): + """Live, a lone "g" serving is stored with defaultMeasureId 0.""" + _, state, fake, result = _create_gram_food(tmp_path) + + assert fake.foods[4242]["defaultMeasureId"] == 0 + assert result == {"food_id": 4242, "measure_id": 9000} + assert state["endpoints"] == ["add_food", "get_food"] + + +def test_update_custom_food_with_zero_default_measure(tmp_path): + client, state, fake, _ = _create_gram_food(tmp_path) + + client.update_custom_food(4242, name="Renamed", calories=3) + + assert state["endpoints"] == ["add_food", "get_food", "get_food", "add_food"] + food = fake.foods[4242] + assert food["name"] == "Renamed" + assert {n["id"]: n["amount"] for n in food["nutrients"]}[208] == 300.0 + assert food["defaultMeasureId"] == 0 + assert [(m["id"], m["name"], m["value"]) for m in food["measures"]] == [ + (9000, "g", 1) + ] + + +@pytest.mark.parametrize("kwargs", [{"name": "Renamed"}, {"calories": 3}]) +def test_update_custom_food_zero_default_with_two_measures_raises(tmp_path, kwargs): + client, state, fake, _ = _create_gram_food(tmp_path) + fake.foods[4242]["measures"].append( + {"id": 9001, "name": "cup", "value": 240, "amount": 1, "type": "Atomic"} + ) + + with pytest.raises(CronometerError, match=r"has 2 measures \['g', 'cup'\]"): + client.update_custom_food(4242, **kwargs) + assert state["endpoints"] == ["add_food", "get_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_recipe_edit.py b/tests/test_recipe_edit.py index e39d55f..63f10c4 100644 --- a/tests/test_recipe_edit.py +++ b/tests/test_recipe_edit.py @@ -1265,23 +1265,50 @@ def test_get_food_details_plain_food_has_no_recipe_fields(monkeypatch): # --------------------------------------------------------------------------- -def test_create_recipe_read_back_without_default_measure_id(tmp_path): +@pytest.mark.parametrize("default", [0, 9001, None]) +def test_create_recipe_finds_serving_whatever_the_default_measure(tmp_path, default): + """The serving is the one non-fixed measure; defaultMeasureId may be 0 or + point at "g" (9001), and the "g" measure must not be renamed.""" fake = FakeFoodServer(ingredient_foods=[EGG]) - client, _ = make_server_client(tmp_path, fake) + client, state = make_server_client(tmp_path, fake) + handle = fake.handle + + def read_back(endpoint, body): + out = handle(endpoint, body) + if endpoint == "/api/v2/get_food": + out["defaultMeasureId"] = default + return out + + fake.handle = read_back + + result = client.create_recipe("R", ingredients=[(1234, 100)]) + + assert result["measure_id"] == 9000 + assert state["endpoints"] == ["get_foods", "add_food", "get_food"] + assert [m["name"] for m in fake.foods[4242]["measures"]] == [ + "Serving", + "g", + "oz", + "full recipe", + ] + + +def test_create_recipe_read_back_with_two_servings(tmp_path): + fake = FakeFoodServer(ingredient_foods=[EGG]) + 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": - out.pop("defaultMeasureId") + out["measures"].append({**out["measures"][0], "id": 1, "name": "bowl"}) return out fake.handle = broken - with pytest.raises( - CronometerError, match=r"exists \(id 4242\).*'defaultMeasureId'" - ): + with pytest.raises(CronometerError, match=r"exists \(id 4242\).*several servings"): client.create_recipe("R", ingredients=[(1234, 100)]) + assert state["endpoints"] == ["get_foods", "add_food", "get_food"] @pytest.mark.parametrize("serving_name", LEADING_QUANTITY_NAMES) From 51e6d309fe1507d28ff736a7172128f1d5baa685 Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Tue, 13 Oct 2026 09:54:04 -0700 Subject: [PATCH 2/2] Let update_custom_food rename a food with an ambiguous default measure Only resolve the default measure when the serving or per-serving nutrients change; a rename never touches it. Co-Authored-By: Claude Opus 5.5 --- src/cronometer_api_mcp/client.py | 7 +++---- tests/test_client.py | 30 ++++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index ec3b0fd..fd97566 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -955,10 +955,9 @@ def update_custom_food( 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) - ): + edits_serving = per_serving or any(v is not None for v in serving_args) + measure = self._default_measure(food) if edits_serving else None + if edits_serving and measure is None: 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 " diff --git a/tests/test_client.py b/tests/test_client.py index c3a581f..8c3488c 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1409,18 +1409,40 @@ def test_update_custom_food_with_zero_default_measure(tmp_path): ] -@pytest.mark.parametrize("kwargs", [{"name": "Renamed"}, {"calories": 3}]) +_CUP = {"id": 9001, "name": "cup", "value": 240, "amount": 1, "type": "Atomic"} + + +@pytest.mark.parametrize("kwargs", [{"calories": 3}, {"serving_grams": 2}]) def test_update_custom_food_zero_default_with_two_measures_raises(tmp_path, kwargs): client, state, fake, _ = _create_gram_food(tmp_path) - fake.foods[4242]["measures"].append( - {"id": 9001, "name": "cup", "value": 240, "amount": 1, "type": "Atomic"} - ) + fake.foods[4242]["measures"].append(dict(_CUP)) with pytest.raises(CronometerError, match=r"has 2 measures \['g', 'cup'\]"): client.update_custom_food(4242, **kwargs) assert state["endpoints"] == ["add_food", "get_food", "get_food"] +def test_update_custom_food_zero_default_with_two_measures_still_renames(tmp_path): + """A rename never touches a measure, so an ambiguous default doesn't matter.""" + client, state, fake, _ = _create_gram_food(tmp_path) + fake.foods[4242]["measures"].append(dict(_CUP)) + + client.update_custom_food(4242, name="Renamed") + + assert state["endpoints"] == ["add_food", "get_food", "get_food", "add_food"] + assert fake.foods[4242]["name"] == "Renamed" + + +def test_update_custom_food_zero_default_with_lone_non_gram_measure(tmp_path): + client, state, fake, _ = _create_gram_food(tmp_path) + fake.foods[4242]["measures"] = [dict(_CUP)] + + client.update_custom_food(4242, calories=120) + + assert state["endpoints"] == ["add_food", "get_food", "get_food", "add_food"] + assert {n["id"]: n["amount"] for n in fake.foods[4242]["nutrients"]}[208] == 50.0 + + 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)