From ffd269b111876c4c223d0b94e47c874b45dc0cde Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Tue, 13 Oct 2026 12:49:11 -0700 Subject: [PATCH 1/3] Retire defaultMeasureId reads: diary entries and recipe ingredients defaultMeasureId is the measure a food was last logged with, not its serving, so nothing reads it any more. enrich_diary_servings no longer falls back to it when an entry's measure is gone. Rules, each verified live against the server's own day totals on a throwaway date with ZZ test foods (since retired): - Weight/Atomic foods: nutrients scale by grams / 100 whatever the measure, including measureId 0 and a deleted measure the server left the entry on. serving_size/servings are set only while the food still has the entry's own measure. - Serving-based recipes (every measure is type Recipe, even "g"): nutrients scale by grams / the measure's value, and grams counts the measure's units. The old code scaled by grams alone, which was right only for value-1 measures; deleting a recipe's measure moves its entries onto "g" (value = grams per reference serving), so it mattered. - A recipe entry whose measure is missing (e.g. logged with measureId 0) raises: the server divided it by a value other than the default measure's, so the old fallback was wrong, and no rule is known. - A food mixing Recipe and other measure types raises. The guard in _food_log_day that undid the fallback goes; _rank_sources tells a recipe entry by servings without a portion weight, so a weight measure without a value no longer nulls the grams sum. _gram_measure_id returns 0 for a food without a 1 g measure: the server accepts it and derives the same batch weight and nutrients as with a real measure (verified live). With #9's get_foods raising on unknown ids, enrich indexes foods directly, and the unknown-food tests use the fake server's real not-found error. Co-Authored-By: Claude Opus 5.5 --- src/cronometer_api_mcp/client.py | 101 ++++++++++++---------- src/cronometer_api_mcp/server.py | 20 ++--- tests/test_client.py | 142 ++++++++++++++++++++++++++++--- tests/test_food_log.py | 55 ++++++++++-- tests/test_nutrient_sources.py | 42 +++++++-- tests/test_recipe_edit.py | 2 +- 6 files changed, 278 insertions(+), 84 deletions(-) diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index a074c53..b7de878 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -627,12 +627,7 @@ def get_food(self, food_id: int) -> dict: """ payload = {"id": food_id, "config": {"call_version": 1}} data = self._request("/api/v2/get_food", payload) - logger.info( - "Fetched food %d: %r (defaultMeasureId=%s)", - food_id, - data.get("name"), - data.get("defaultMeasureId"), - ) + logger.info("Fetched food %d: %r", food_id, data.get("name")) return data def get_foods(self, food_ids: list[int]) -> list[dict]: @@ -1985,17 +1980,29 @@ def enrich_diary_servings(self, diary: dict) -> dict: - name: from the food object - serving_size: {measure_id, quantity, unit, grams} defining one - portion in the entry's measureId (falls back to defaultMeasureId). - Recipe measures have null grams: their value is a reference-serving - count, not a gram weight. + portion in the entry's measureId, only while the food still has + that measure. Recipe measures have null grams (see below). - servings: number of those portions consumed, when derivable - - nutrients: the food's nutrient profile scaled to the entry's amount - (per-100g for Weight/Atomic measures, per-serving for Recipe - measures), as {nutrient_id: amount} - - A get_foods failure raises. If a food is not - returned, the corresponding entries are left unchanged. The diary dict - is mutated in place and also returned. Non-Serving entries (Exercise, + - nutrients: the food's nutrient profile scaled to the entry's + amount, as {nutrient_id: amount} + + How "grams" reads depends on the food (all verified live against the + server's own day totals): + - Weight/Atomic measures: real grams. Nutrients are per 100 g, so + they scale by grams / 100 whatever the measure, even one the food + no longer has (measureId 0, or a deleted measure the server left + the entry on). + - Recipe measures (serving-based recipes made in the app): "grams" is + a count of the measure's units, and the measure's value is how many + units make one reference serving, which the nutrients are stored + per. So nutrients scale by grams / value, and servings is grams. + Deleting a measure moves its entries onto the recipe's "g" + measure. An entry whose measure is missing anyway (e.g. logged + with measureId 0) can't be scaled: the server then divides by a + value that isn't the default measure's, so this raises. + + A get_foods failure raises, including an unknown food id. The diary + dict is mutated in place and also returned. Non-Serving entries (Exercise, Biometric) already carry a name and are left untouched. """ if not isinstance(diary, dict): @@ -2022,19 +2029,32 @@ def enrich_diary_servings(self, diary: dict) -> dict: for entry in entries: if not isinstance(entry, dict) or entry.get("type") != "Serving": continue - food = food_by_id.get(entry.get("foodId")) - if not food: - continue + food = food_by_id[entry["foodId"]] entry["name"] = food.get("name") - measures = { - m.get("id"): m for m in food.get("measures", []) if isinstance(m, dict) - } - measure = measures.get(entry.get("measureId")) or measures.get( - food.get("defaultMeasureId") + measures = [m for m in food.get("measures", []) if isinstance(m, dict)] + measure = next( + (m for m in measures if m.get("id") == entry.get("measureId")), None ) + # The server makes every measure of a serving-based recipe, even + # its "g", type Recipe (seen live); a mix has no known reading. + recipe_flags = {m.get("type") == "Recipe" for m in measures} + recipe = recipe_flags == {True} + if len(recipe_flags) > 1: + raise CronometerError( + f"Food {food.get('id')} ({food.get('name')!r}) mixes Recipe " + f"and other measure types, so its diary amounts can't be read" + ) grams = entry.get("grams") + if recipe and not (measure and _is_positive_number(measure.get("value"))): + raise CronometerError( + f"Diary entry {entry.get('servingId')} ({food.get('name')!r}, " + f"a serving-based recipe) is logged with measure " + f"{entry.get('measureId')}, which is missing or has no " + f"usable value, so its amount can't be read; re-log it in " + f"the Cronometer app" + ) if measure: portion_value = measure.get("value") quantity = measure.get("amount", 1) @@ -2044,26 +2064,17 @@ def enrich_diary_servings(self, diary: dict) -> dict: "measure_id": measure.get("id"), "quantity": quantity, "unit": measure.get("name"), - "grams": None if measure.get("type") == "Recipe" else portion_value, + "grams": None if recipe else portion_value, } - if ( - isinstance(grams, (int, float)) - and isinstance(portion_value, (int, float)) - and portion_value + if isinstance(grams, (int, float)) and _is_positive_number( + portion_value ): - entry["servings"] = round(grams / portion_value, 4) - - # Nutrient scaling depends on the measure type: - # - Recipe measures: nutrients are stored per one reference - # serving and the diary "grams" field is a serving count, so - # scale by grams directly. - # - Weight/Atomic measures: nutrients are stored per-100g and - # "grams" is real grams, so scale by grams / 100. + entry["servings"] = round( + grams if recipe else grams / portion_value, 4 + ) + if isinstance(grams, (int, float)): - if measure and measure.get("type") == "Recipe": - scale = grams - else: - scale = grams / 100.0 + scale = grams / (measure["value"] if recipe else 100.0) entry["nutrients"] = { n["id"]: round(n["amount"] * scale, 4) for n in food.get("nutrients", []) @@ -2391,14 +2402,14 @@ def _gram_measure_id(food: dict) -> int: Ingredient measures are display metadata -- Cronometer renders the amount as grams / measure value -- so the 1-gram measure makes the UI show the - gram weight directly. Falls back to the food's default measure when no - gram measure exists, since `grams` is what the nutrient math uses either - way. + gram weight directly. A food without one gets 0: the server accepts it and + derives the same nutrients and batch weight as from a real measure + (verified live), since `grams` is what the nutrient math uses. """ for m in food.get("measures", []): if isinstance(m, dict) and m.get("name") == "g" and m.get("value") == 1: return m["id"] - return food.get("defaultMeasureId") or 0 + return 0 def _translation_id(food: dict) -> int: diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index 561a266..a1e9395 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -289,13 +289,11 @@ def _food_log_day( picked = {names[nid]: amounts[nid] for nid in nutrient_ids if nid in amounts} for name, amount in picked.items(): totals[name] += amount + # enrich_diary_servings sets serving_size only while the food still has + # the logged measure, servings only when that measure has a value, and + # gives a serving-based recipe's measure null grams. size = entry.get("serving_size") - # enrich_diary_servings falls back to the default measure when the - # logged one is gone; that measure would misdescribe the amount. - logged = size is not None and size["measure_id"] == entry["measureId"] - # enrich_diary_servings sets servings only when the measure has a - # weight, and gives a serving-based recipe's measure null grams. - counted = logged and "servings" in entry + counted = "servings" in entry recipe = counted and size["grams"] is None quantity = size["quantity"] if counted else None entries.append( @@ -306,7 +304,7 @@ def _food_log_day( "food_id": entry["foodId"], "measure_id": entry["measureId"], "grams": None if recipe else entry["grams"], - "measure": size["unit"] if logged else None, + "measure": size["unit"] if size else None, "quantity": None if quantity is None else round(quantity * entry["servings"], 2), @@ -419,7 +417,7 @@ def _food_servings(*diaries: dict) -> list[dict]: """The Serving entries of enriched diaries. Raises if any lacks nutrient data (enrichment leaves an entry bare when - get_foods doesn't return its food), since totals would silently undercount. + its grams isn't a number), since totals would silently undercount. """ entries = [_diary_entries(diary) for diary in diaries] servings = [e for day in entries for e in day if e["type"] == "Serving"] @@ -486,10 +484,10 @@ def _rank_sources( ) food["amount"] += amount food["entries"] += 1 - # Serving-based recipe entries count reference servings, not grams. - serving_size = entry.get("serving_size") + # Serving-based recipe entries count measure units, not grams; only + # they have servings without a portion weight. if food["grams"] is not None and ( - serving_size is None or serving_size["grams"] is not None + "servings" not in entry or entry["serving_size"]["grams"] is not None ): food["grams"] += entry["grams"] else: diff --git a/tests/test_client.py b/tests/test_client.py index 5b919fe..7d82e0b 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -481,7 +481,7 @@ def _enrich_client(tmp_path: Path): { "type": "Serving", "foodId": 200, - "measureId": 999, # unknown measure -> falls back to default + "measureId": 999, # a measure the food no longer has "grams": 50, "order": 65538, }, @@ -553,10 +553,10 @@ def test_enrich_diary_merges_names_measures_and_scaled_nutrients(tmp_path): assert oats["nutrients"] == {208: 778.0, 203: 33.8} milk = entries[1] - # unknown measureId 999 -> fell back to defaultMeasureId 20 - assert milk["serving_size"]["measure_id"] == 20 - assert milk["serving_size"]["unit"] == "glass" - # 42 kcal/100g scaled to 50g -> 21 + # Measure 999 is gone: no serving is described (not defaultMeasureId 20's + # "glass"), but grams are real grams, so 42 kcal/100g scaled to 50g -> 21. + assert "serving_size" not in milk + assert "servings" not in milk assert milk["nutrients"] == {208: 21.0} recipe = entries[2] @@ -603,7 +603,7 @@ def test_enrich_diary_measure_quantity( monkeypatch.setattr(client, "get_foods", lambda ids: [food]) out = client.enrich_diary_servings( - {"diary": [{"type": "Serving", "foodId": 100, "measureId": 999, "grams": 60}]} + {"diary": [{"type": "Serving", "foodId": 100, "measureId": 10, "grams": 60}]} ) entry = out["diary"][0] @@ -612,6 +612,121 @@ def test_enrich_diary_measure_quantity( assert entry["servings"] == servings +# The serving-based recipe below is the one probed live (ZZ test, retired): +# 200 g of egg, 71.5 kcal stored per reference serving. After its "serving" +# measure was deleted, the server re-created the "g" measure and moved the +# entry logged by "serving" onto it. Each entry's expected energy is what the +# server's own day total dropped by when that entry was deleted. +LIVE_SERVING_RECIPE = { + "id": 85100983, + "name": "ZZ test serving recipe", + "defaultMeasureId": 313595231, + "measures": [ + {"id": 313595763, "name": "g", "value": 200, "amount": 1, "type": "Recipe"}, + {"id": 313595231, "name": "full recipe", "value": 4, "amount": 1, + "type": "Recipe"}, + {"id": 313595764, "name": "portion", "value": 1, "amount": 1, + "type": "Recipe"}, + ], + "nutrients": [{"id": 208, "amount": 71.5}], +} # fmt: skip + + +def _enrich_one(tmp_path, food, measure_id, grams): + client = _enrich_client(tmp_path) + client.get_foods = lambda ids: [food] # type: ignore[method-assign] + entry = { + "type": "Serving", + "servingId": 7, + "foodId": food["id"], + "measureId": measure_id, + "grams": grams, + } + return client.enrich_diary_servings({"diary": [entry]})["diary"][0] + + +@pytest.mark.parametrize( + ("measure_id", "grams", "server_kcal"), + [ + (313595763, 1.5, 0.5362), # moved onto "g" when "serving" was deleted + (313595231, 4, 71.5), # "full recipe", value 4 + (313595764, 2, 143.0), # value 1: grams is a serving count + ], +) +def test_enrich_recipe_scales_by_the_measure_value_like_the_server( + tmp_path, measure_id, grams, server_kcal +): + entry = _enrich_one(tmp_path, LIVE_SERVING_RECIPE, measure_id, grams) + + assert entry["nutrients"][208] == pytest.approx(server_kcal, abs=1e-4) + assert entry["serving_size"]["grams"] is None + # grams counts the measure's units. + assert entry["servings"] == grams + + +@pytest.mark.parametrize("measure_id", [0, 999, 313595230]) +def test_enrich_recipe_entry_without_its_measure_raises(tmp_path, measure_id): + # Live, a measure-0 entry (2) counted 0.715 kcal: 71.5 * 2 / 200, not the + # default measure's 71.5 * 2 / 4. With no rule for which value the server + # picks, the amount is refused rather than guessed. + with pytest.raises(CronometerError, match=f"measure {measure_id}, which"): + _enrich_one(tmp_path, LIVE_SERVING_RECIPE, measure_id, 2) + + +@pytest.mark.parametrize("value", [0, -1, None, "1", float("nan"), float("inf")]) +def test_enrich_recipe_measure_without_a_usable_value_raises(tmp_path, value): + food = { + **LIVE_SERVING_RECIPE, + "measures": [{"id": 1, "name": "bowl", "value": value, "type": "Recipe"}], + } + + with pytest.raises(CronometerError, match="no usable value"): + _enrich_one(tmp_path, food, 1, 2) + + +def test_enrich_mixed_recipe_and_weight_measures_raises(tmp_path): + food = { + **LIVE_SERVING_RECIPE, + "measures": [ + *LIVE_SERVING_RECIPE["measures"], + {"id": 5, "name": "oz", "value": 28.35, "type": "Weight"}, + ], + } + + with pytest.raises(CronometerError, match="mixes Recipe"): + _enrich_one(tmp_path, food, 5, 28.35) + + +@pytest.mark.parametrize("measure_id", [0, 999]) +@pytest.mark.parametrize("default", [0, 10, 999]) +def test_enrich_weight_entry_without_its_measure_ignores_the_default( + tmp_path, measure_id, default +): + # Live: an Atomic custom food's measure-0 entry (30 g) and its entry left on + # a deleted measure (100 g) counted 120 and 400 kcal, i.e. per 100 g. + food = { + "id": 100, + "name": "Bar", + "defaultMeasureId": default, + "measures": [{"id": 10, "name": "chunk", "value": 40, "type": "Atomic"}], + "nutrients": [{"id": 208, "amount": 400}], + } + + entry = _enrich_one(tmp_path, food, measure_id, 30) + + assert "serving_size" not in entry + assert "servings" not in entry + assert entry["nutrients"] == {208: 120.0} + + +def test_enrich_recipe_entry_ignores_the_default_measure(tmp_path): + # The default is "full recipe" (value 4); the entry's own measure counts. + entry = _enrich_one(tmp_path, LIVE_SERVING_RECIPE, 313595764, 1) + + assert entry["serving_size"]["unit"] == "portion" + assert entry["nutrients"] == {208: 71.5} + + def test_enrich_diary_raises_when_get_foods_fails(tmp_path): client = _enrich_client(tmp_path) @@ -626,16 +741,15 @@ def boom(ids): ) -def test_enrich_diary_leaves_foods_get_foods_omits_bare(tmp_path): - client = _enrich_client(tmp_path) - client.get_foods = lambda ids: SAMPLE_FOODS[1:] # type: ignore[method-assign] - - out = client.enrich_diary_servings( - {"diary": [dict(e) for e in SAMPLE_DIARY["diary"]]} +def test_enrich_diary_unknown_food_raises_the_not_found_error(tmp_path): + client, _ = make_server_client( + tmp_path, FakeFoodServer(ingredient_foods=SAMPLE_FOODS[1:]) ) - assert out["diary"][0] == SAMPLE_DIARY["diary"][0] - assert out["diary"][1]["nutrients"] == {208: 21.0} + with pytest.raises(CronometerError, match=r"doGetFoods\(\): Food 100 not found$"): + client.enrich_diary_servings( + {"diary": [dict(e) for e in SAMPLE_DIARY["diary"]]} + ) # --------------------------------------------------------------------------- diff --git a/tests/test_food_log.py b/tests/test_food_log.py index 33f06c5..3667c6f 100644 --- a/tests/test_food_log.py +++ b/tests/test_food_log.py @@ -9,6 +9,7 @@ from unittest.mock import Mock, call import pytest +from test_client import FakeFoodServer, make_server_client from cronometer_api_mcp import server from cronometer_api_mcp.client import CronometerClient @@ -54,7 +55,10 @@ "id": 2, "name": "Stew", "measures": [ - {"id": 20, "name": "bowl", "value": 1, "amount": 1, "type": "Recipe"} + {"id": 20, "name": "bowl", "value": 1, "amount": 1, "type": "Recipe"}, + # A serving-based recipe's "g" is a Recipe measure valued at the + # grams in one reference serving. + {"id": 21, "name": "g", "value": 250, "amount": 1, "type": "Recipe"}, ], "nutrients": [{"id": ENERGY, "amount": 400}, {"id": PROTEIN, "amount": 30}], }, @@ -193,13 +197,34 @@ def test_serving_based_recipe_has_null_grams_and_a_count(client): assert entry["nutrients"] == {"Energy": 600.0, "Protein": 45.0} -def test_a_measure_the_food_no_longer_has_is_null_not_the_default(client): - # enrich_diary_servings describes it with the default measure ("large"). - client.diaries[date(2026, 10, 14)] = _diary(_serving(7, 1, 999, 100, 2)) +def test_serving_based_recipe_logged_in_grams(client): + client.diaries[date(2026, 10, 14)] = _diary(_serving(7, 2, 21, 300, 3)) (entry,) = _call()["entries"] - assert entry["measure_id"] == 999 + assert entry["grams"] is None + assert (entry["measure"], entry["quantity"]) == ("g", 300) + assert entry["nutrients"] == {"Energy": 480.0, "Protein": 36.0} + + +@pytest.mark.parametrize("measure_id", [0, 999]) +def test_serving_based_recipe_without_its_measure_is_an_error(client, measure_id): + client.diaries[date(2026, 10, 14)] = _diary(_serving(7, 2, measure_id, 2, 3)) + + result = _call() + + assert result["status"] == "error" + assert "Diary entry 7 ('Stew', a serving-based recipe)" in result["message"] + + +@pytest.mark.parametrize("measure_id", [0, 999]) +def test_a_measure_the_food_no_longer_has_is_null_not_the_default(client, measure_id): + # The egg's defaultMeasureId (10, "large") must not describe it. + client.diaries[date(2026, 10, 14)] = _diary(_serving(7, 1, measure_id, 100, 2)) + + (entry,) = _call()["entries"] + + assert entry["measure_id"] == measure_id assert entry["grams"] == 100 assert entry["measure"] is None assert entry["quantity"] is None @@ -332,14 +357,30 @@ def test_failed_food_lookup_reports_its_cause(client): assert _call() == {"status": "error", "message": "RuntimeError: boom"} -def test_food_missing_from_lookup_is_an_error_not_a_bare_entry(client): - client.get_foods.side_effect = lambda ids: [FOODS[i] for i in ids if i != 1] +def test_unknown_food_is_the_servers_not_found_error(client, tmp_path): + # A real client against the fake server, which answers an unknown id with + # the live-captured FAIL; food 2 is known, food 1 isn't. + real, _ = make_server_client(tmp_path, FakeFoodServer(ingredient_foods=[FOODS[2]])) + client.get_foods.side_effect = real.get_foods client.diaries[date(2026, 10, 14)] = _diary( _serving(7, 1, 10, 50, 1), _serving(8, 2, 20, 1, 3) ) result = _call() + assert result == { + "status": "error", + "message": "CronometerError: Cronometer API error: WebServiceV2 doGetFoods(): Food 1 not found", + } + + +def test_entry_without_numeric_grams_is_an_error_not_a_bare_entry(client): + client.diaries[date(2026, 10, 14)] = _diary( + _serving(7, 1, 10, None, 1), _serving(8, 2, 20, 1, 3) + ) + + result = _call() + assert result["status"] == "error" assert "food_ids 1;" in result["message"] diff --git a/tests/test_nutrient_sources.py b/tests/test_nutrient_sources.py index ce3454c..f3df964 100644 --- a/tests/test_nutrient_sources.py +++ b/tests/test_nutrient_sources.py @@ -11,6 +11,7 @@ from unittest.mock import Mock, call import pytest +from test_client import FakeFoodServer, make_server_client from cronometer_api_mcp import server from cronometer_api_mcp.client import CronometerClient @@ -376,9 +377,11 @@ def test_serving_based_recipe_scales_per_serving_and_has_no_grams(client): assert sodium["sources"][1]["grams"] == 100 -def test_recipe_grams_stay_null_when_mixed_with_weight_entries(client): +def test_recipe_grams_stay_null_when_logged_by_its_g_measure(client): + # A serving-based recipe's "g" is a Recipe measure too, valued at the + # grams in one reference serving (seen live). weighed = _food(5, "Stew", {SODIUM: 900}, measure_type="Recipe", value=1) - weighed["measures"].append({"id": 51, "name": "g", "value": 1, "type": "Weight"}) + weighed["measures"].append({"id": 51, "name": "g", "value": 250, "type": "Recipe"}) foods_with_grams = {**FOODS, 5: weighed} client.get_foods.side_effect = lambda ids: [foods_with_grams[i] for i in ids] by_weight = {**_serving(5, 100, 3), "measureId": 51} @@ -388,6 +391,29 @@ def test_recipe_grams_stay_null_when_mixed_with_weight_entries(client): assert stew["grams"] is None assert stew["entries"] == 2 + assert stew["amount"] == 900 + 900 * 100 / 250 + + +@pytest.mark.parametrize("value", [None, 0, "100"]) +def test_weight_entries_keep_grams_when_their_measure_has_no_value(client, value): + bread = _food(1, "Bread", {SODIUM: 500}, value=value) + client.get_foods.side_effect = lambda ids: [bread] + client.diaries[date(2026, 10, 14)] = _diary(_serving(1, 40, 1)) + + (source,) = _only(_call(nutrients="sodium"))["sources"] + + assert (source["grams"], source["amount"]) == (40, 200) + + +@pytest.mark.parametrize("measure_id", [0, 999]) +def test_weight_entries_without_their_measure_keep_grams(client, measure_id): + client.diaries[date(2026, 10, 14)] = _diary( + {**_serving(1, 40, 1), "measureId": measure_id} + ) + + (source,) = _only(_call(nutrients="sodium"))["sources"] + + assert (source["grams"], source["amount"]) == (40, 200) def test_unknown_diary_group_is_labelled_by_number(client): @@ -407,14 +433,18 @@ def test_failed_food_lookup_reports_its_cause(client): assert result == {"status": "error", "message": "RuntimeError: boom"} -def test_food_missing_from_lookup_is_an_error(client): - client.get_foods.side_effect = lambda ids: [FOODS[i] for i in ids if i != 2] +def test_unknown_food_is_the_servers_not_found_error(client, tmp_path): + real, _ = make_server_client(tmp_path, FakeFoodServer(ingredient_foods=[FOODS[1]])) + client.get_foods.side_effect = real.get_foods client.diaries[date(2026, 10, 14)] = _diary(_serving(1, 100, 1), _serving(2, 10, 1)) result = _call(nutrients="sodium") - assert result["status"] == "error" - assert "food_ids 2;" in result["message"] + assert result == { + "status": "error", + "message": "CronometerError: Cronometer API error: " + "WebServiceV2 doGetFoods(): Food 2 not found", + } # --------------------------------------------------------------------------- diff --git a/tests/test_recipe_edit.py b/tests/test_recipe_edit.py index 376a6dc..eb89350 100644 --- a/tests/test_recipe_edit.py +++ b/tests/test_recipe_edit.py @@ -552,7 +552,7 @@ def test_ingredients_replace_list_leaving_derivation_to_server(tmp_path): { "id": 0, "foodId": 619, - "measureId": 1960, # no gram measure: default measure + "measureId": 0, # no gram measure (the server accepts 0) "translationId": 0, "grams": 16.0, "value": 16.0, From 2696f7804aa390ba73279b48278bed1537fbbb3d Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Tue, 13 Oct 2026 12:52:30 -0700 Subject: [PATCH 2/3] Trim comments Co-Authored-By: Claude Opus 5.5 --- src/cronometer_api_mcp/client.py | 21 +++++---------------- src/cronometer_api_mcp/server.py | 7 ++----- tests/test_client.py | 18 ++++-------------- tests/test_food_log.py | 4 ---- tests/test_nutrient_sources.py | 2 -- 5 files changed, 11 insertions(+), 41 deletions(-) diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index b7de878..a8925e4 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -1986,20 +1986,11 @@ def enrich_diary_servings(self, diary: dict) -> dict: - nutrients: the food's nutrient profile scaled to the entry's amount, as {nutrient_id: amount} - How "grams" reads depends on the food (all verified live against the - server's own day totals): - - Weight/Atomic measures: real grams. Nutrients are per 100 g, so - they scale by grams / 100 whatever the measure, even one the food - no longer has (measureId 0, or a deleted measure the server left - the entry on). - - Recipe measures (serving-based recipes made in the app): "grams" is - a count of the measure's units, and the measure's value is how many - units make one reference serving, which the nutrients are stored - per. So nutrients scale by grams / value, and servings is grams. - Deleting a measure moves its entries onto the recipe's "g" - measure. An entry whose measure is missing anyway (e.g. logged - with measureId 0) can't be scaled: the server then divides by a - value that isn't the default measure's, so this raises. + Scaling matches the server's day totals (verified live): Weight/Atomic + foods by grams / 100, whatever the measure; serving-based recipes (all + measures type Recipe) by grams / the measure's value, with grams a + count of its units. A recipe entry without its measure raises: the + server's divisor then isn't the default measure's. A get_foods failure raises, including an unknown food id. The diary dict is mutated in place and also returned. Non-Serving entries (Exercise, @@ -2037,8 +2028,6 @@ def enrich_diary_servings(self, diary: dict) -> dict: measure = next( (m for m in measures if m.get("id") == entry.get("measureId")), None ) - # The server makes every measure of a serving-based recipe, even - # its "g", type Recipe (seen live); a mix has no known reading. recipe_flags = {m.get("type") == "Recipe" for m in measures} recipe = recipe_flags == {True} if len(recipe_flags) > 1: diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index a1e9395..950d6f6 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -289,9 +289,7 @@ def _food_log_day( picked = {names[nid]: amounts[nid] for nid in nutrient_ids if nid in amounts} for name, amount in picked.items(): totals[name] += amount - # enrich_diary_servings sets serving_size only while the food still has - # the logged measure, servings only when that measure has a value, and - # gives a serving-based recipe's measure null grams. + # Only a serving-based recipe has servings but no portion weight. size = entry.get("serving_size") counted = "servings" in entry recipe = counted and size["grams"] is None @@ -484,8 +482,7 @@ def _rank_sources( ) food["amount"] += amount food["entries"] += 1 - # Serving-based recipe entries count measure units, not grams; only - # they have servings without a portion weight. + # Serving-based recipe entries count measure units, not grams. if food["grams"] is not None and ( "servings" not in entry or entry["serving_size"]["grams"] is not None ): diff --git a/tests/test_client.py b/tests/test_client.py index 7d82e0b..470a943 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -553,8 +553,7 @@ def test_enrich_diary_merges_names_measures_and_scaled_nutrients(tmp_path): assert oats["nutrients"] == {208: 778.0, 203: 33.8} milk = entries[1] - # Measure 999 is gone: no serving is described (not defaultMeasureId 20's - # "glass"), but grams are real grams, so 42 kcal/100g scaled to 50g -> 21. + # Measure 999 is gone: no serving (not the default's), still per 100 g. assert "serving_size" not in milk assert "servings" not in milk assert milk["nutrients"] == {208: 21.0} @@ -612,11 +611,7 @@ def test_enrich_diary_measure_quantity( assert entry["servings"] == servings -# The serving-based recipe below is the one probed live (ZZ test, retired): -# 200 g of egg, 71.5 kcal stored per reference serving. After its "serving" -# measure was deleted, the server re-created the "g" measure and moved the -# entry logged by "serving" onto it. Each entry's expected energy is what the -# server's own day total dropped by when that entry was deleted. +# The serving-based recipe probed live; expected kcal are the server's own. LIVE_SERVING_RECIPE = { "id": 85100983, "name": "ZZ test serving recipe", @@ -660,15 +655,12 @@ def test_enrich_recipe_scales_by_the_measure_value_like_the_server( assert entry["nutrients"][208] == pytest.approx(server_kcal, abs=1e-4) assert entry["serving_size"]["grams"] is None - # grams counts the measure's units. assert entry["servings"] == grams @pytest.mark.parametrize("measure_id", [0, 999, 313595230]) def test_enrich_recipe_entry_without_its_measure_raises(tmp_path, measure_id): - # Live, a measure-0 entry (2) counted 0.715 kcal: 71.5 * 2 / 200, not the - # default measure's 71.5 * 2 / 4. With no rule for which value the server - # picks, the amount is refused rather than guessed. + # Live, measure 0 counted 71.5 * 2 / 200, not the default's 71.5 * 2 / 4. with pytest.raises(CronometerError, match=f"measure {measure_id}, which"): _enrich_one(tmp_path, LIVE_SERVING_RECIPE, measure_id, 2) @@ -702,8 +694,7 @@ def test_enrich_mixed_recipe_and_weight_measures_raises(tmp_path): def test_enrich_weight_entry_without_its_measure_ignores_the_default( tmp_path, measure_id, default ): - # Live: an Atomic custom food's measure-0 entry (30 g) and its entry left on - # a deleted measure (100 g) counted 120 and 400 kcal, i.e. per 100 g. + # Live, such entries counted per 100 g. food = { "id": 100, "name": "Bar", @@ -720,7 +711,6 @@ def test_enrich_weight_entry_without_its_measure_ignores_the_default( def test_enrich_recipe_entry_ignores_the_default_measure(tmp_path): - # The default is "full recipe" (value 4); the entry's own measure counts. entry = _enrich_one(tmp_path, LIVE_SERVING_RECIPE, 313595764, 1) assert entry["serving_size"]["unit"] == "portion" diff --git a/tests/test_food_log.py b/tests/test_food_log.py index 3667c6f..3bfb1a8 100644 --- a/tests/test_food_log.py +++ b/tests/test_food_log.py @@ -56,8 +56,6 @@ "name": "Stew", "measures": [ {"id": 20, "name": "bowl", "value": 1, "amount": 1, "type": "Recipe"}, - # A serving-based recipe's "g" is a Recipe measure valued at the - # grams in one reference serving. {"id": 21, "name": "g", "value": 250, "amount": 1, "type": "Recipe"}, ], "nutrients": [{"id": ENERGY, "amount": 400}, {"id": PROTEIN, "amount": 30}], @@ -358,8 +356,6 @@ def test_failed_food_lookup_reports_its_cause(client): def test_unknown_food_is_the_servers_not_found_error(client, tmp_path): - # A real client against the fake server, which answers an unknown id with - # the live-captured FAIL; food 2 is known, food 1 isn't. real, _ = make_server_client(tmp_path, FakeFoodServer(ingredient_foods=[FOODS[2]])) client.get_foods.side_effect = real.get_foods client.diaries[date(2026, 10, 14)] = _diary( diff --git a/tests/test_nutrient_sources.py b/tests/test_nutrient_sources.py index f3df964..417300e 100644 --- a/tests/test_nutrient_sources.py +++ b/tests/test_nutrient_sources.py @@ -378,8 +378,6 @@ def test_serving_based_recipe_scales_per_serving_and_has_no_grams(client): def test_recipe_grams_stay_null_when_logged_by_its_g_measure(client): - # A serving-based recipe's "g" is a Recipe measure too, valued at the - # grams in one reference serving (seen live). weighed = _food(5, "Stew", {SODIUM: 900}, measure_type="Recipe", value=1) weighed["measures"].append({"id": 51, "name": "g", "value": 250, "type": "Recipe"}) foods_with_grams = {**FOODS, 5: weighed} From 48ccf3aaf572729efa27448153dae730cbdceb8c Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Tue, 13 Oct 2026 12:56:19 -0700 Subject: [PATCH 3/3] Raise on non-numeric grams in enrich; flag recipe entries; fix stale docs Review fixes: - enrich_diary_servings raises on a Serving entry whose grams isn't a finite number, naming the entry, so _food_servings' "Try again" re-check (which could no longer be a lookup failure) goes. - enrich marks each entry recipe True/False; _food_log_day and _rank_sources read it instead of inferring it from serving_size. - get_food_log, get_nutrient_sources and create_recipe docstrings now say a serving-based recipe's grams counts the logged measure's units. Co-Authored-By: Claude Opus 5.5 --- src/cronometer_api_mcp/client.py | 33 ++++++++++++++++++-------------- src/cronometer_api_mcp/server.py | 33 ++++++++------------------------ tests/test_client.py | 12 ++++++++++++ tests/test_food_log.py | 9 ++++++--- 4 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index a8925e4..c932176 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -1059,10 +1059,10 @@ def create_recipe( This creates a *weight-based* recipe: measures are type "Weight", and the nutrients the server derives are stored per-100g. - (Cronometer's other mode, "serving-based", uses type "Recipe" measures - and stores nutrients per full batch, which makes the diary's `grams` - field a serving count instead of real grams -- see - enrich_diary_servings. Weight-based avoids that quirk entirely.) + (Cronometer's other mode, "serving-based", uses type "Recipe" measures, + stores nutrients per reference serving, and makes the diary's `grams` + field a count of measure units -- see enrich_diary_servings. + Weight-based avoids that quirk entirely.) Args: name: Recipe name. @@ -1983,6 +1983,7 @@ def enrich_diary_servings(self, diary: dict) -> dict: portion in the entry's measureId, only while the food still has that measure. Recipe measures have null grams (see below). - servings: number of those portions consumed, when derivable + - recipe: whether the food is a serving-based recipe - nutrients: the food's nutrient profile scaled to the entry's amount, as {nutrient_id: amount} @@ -1990,7 +1991,8 @@ def enrich_diary_servings(self, diary: dict) -> dict: foods by grams / 100, whatever the measure; serving-based recipes (all measures type Recipe) by grams / the measure's value, with grams a count of its units. A recipe entry without its measure raises: the - server's divisor then isn't the default measure's. + server's divisor then isn't the default measure's. So does an entry + whose grams isn't a number. A get_foods failure raises, including an unknown food id. The diary dict is mutated in place and also returned. Non-Serving entries (Exercise, @@ -2036,6 +2038,12 @@ def enrich_diary_servings(self, diary: dict) -> dict: f"and other measure types, so its diary amounts can't be read" ) grams = entry.get("grams") + if not (isinstance(grams, (int, float)) and math.isfinite(grams)): + raise CronometerError( + f"Diary entry {entry.get('servingId')} ({food.get('name')!r}) " + f"has grams {grams!r}, not a number" + ) + entry["recipe"] = recipe if recipe and not (measure and _is_positive_number(measure.get("value"))): raise CronometerError( f"Diary entry {entry.get('servingId')} ({food.get('name')!r}, " @@ -2055,19 +2063,16 @@ def enrich_diary_servings(self, diary: dict) -> dict: "unit": measure.get("name"), "grams": None if recipe else portion_value, } - if isinstance(grams, (int, float)) and _is_positive_number( - portion_value - ): + if _is_positive_number(portion_value): entry["servings"] = round( grams if recipe else grams / portion_value, 4 ) - if isinstance(grams, (int, float)): - scale = grams / (measure["value"] if recipe else 100.0) - entry["nutrients"] = { - n["id"]: round(n["amount"] * scale, 4) - for n in food.get("nutrients", []) - } + scale = grams / (measure["value"] if recipe else 100.0) + entry["nutrients"] = { + n["id"]: round(n["amount"] * scale, 4) + for n in food.get("nutrients", []) + } logger.info("Enriched %d diary foods with names/nutrients", len(food_by_id)) return diary diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index 950d6f6..bf3efd0 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -233,7 +233,7 @@ def get_food_log( even if renamed in the app), or "group N" for any further group - food, food_id, measure_id: what add_food_entries takes to re-log it - grams: the logged weight; null for a serving-based recipe, whose - diary amount is a count of reference servings (quantity holds it) + diary amount counts the logged measure's units (quantity holds it) - measure and quantity: the amount in the logged measure, e.g. 3 "large"; both null if the food no longer has that measure, and quantity null if the measure has no usable weight @@ -289,11 +289,8 @@ def _food_log_day( picked = {names[nid]: amounts[nid] for nid in nutrient_ids if nid in amounts} for name, amount in picked.items(): totals[name] += amount - # Only a serving-based recipe has servings but no portion weight. size = entry.get("serving_size") - counted = "servings" in entry - recipe = counted and size["grams"] is None - quantity = size["quantity"] if counted else None + quantity = size["quantity"] if "servings" in entry else None entries.append( { "serving_id": entry["servingId"], @@ -301,7 +298,7 @@ def _food_log_day( "food": entry["name"], "food_id": entry["foodId"], "measure_id": entry["measureId"], - "grams": None if recipe else entry["grams"], + "grams": None if entry["recipe"] else entry["grams"], "measure": size["unit"] if size else None, "quantity": None if quantity is None @@ -342,8 +339,8 @@ def get_nutrient_sources( matches get_daily_nutrition. Exercise and biometric entries are ignored, and a food with no data for a nutrient adds nothing to it. - grams is null for a food logged in a serving-based recipe's reference - servings, which have no gram weight. Diary groups 1-4 use Cronometer's + grams is null for a serving-based recipe, whose diary amounts count + measure units, not grams. Diary groups 1-4 use Cronometer's default names even if they were renamed in the app; any further group shows as "group N". @@ -412,21 +409,9 @@ def _diary_entries(diary: dict) -> list[dict]: def _food_servings(*diaries: dict) -> list[dict]: - """The Serving entries of enriched diaries. - - Raises if any lacks nutrient data (enrichment leaves an entry bare when - its grams isn't a number), since totals would silently undercount. - """ + """The Serving entries of enriched diaries.""" entries = [_diary_entries(diary) for diary in diaries] - servings = [e for day in entries for e in day if e["type"] == "Serving"] - unresolved = sorted({e["foodId"] for e in servings if "nutrients" not in e}) - if unresolved: - raise RuntimeError( - "Could not load nutrient data for food_ids " - + ", ".join(map(str, unresolved)) - + "; totals would be incomplete. Try again." - ) - return servings + return [e for day in entries for e in day if e["type"] == "Serving"] def _match_nutrients(queries: list[str], defs: dict[int, dict]) -> list[int]: @@ -483,9 +468,7 @@ def _rank_sources( food["amount"] += amount food["entries"] += 1 # Serving-based recipe entries count measure units, not grams. - if food["grams"] is not None and ( - "servings" not in entry or entry["serving_size"]["grams"] is not None - ): + if food["grams"] is not None and not entry["recipe"]: food["grams"] += entry["grams"] else: food["grams"] = None diff --git a/tests/test_client.py b/tests/test_client.py index 470a943..6836e17 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -655,6 +655,7 @@ def test_enrich_recipe_scales_by_the_measure_value_like_the_server( assert entry["nutrients"][208] == pytest.approx(server_kcal, abs=1e-4) assert entry["serving_size"]["grams"] is None + assert entry["recipe"] is True assert entry["servings"] == grams @@ -710,6 +711,17 @@ def test_enrich_weight_entry_without_its_measure_ignores_the_default( assert entry["nutrients"] == {208: 120.0} +@pytest.mark.parametrize("measures", [[], [{"id": 10, "name": "x", "value": 40}]]) +def test_enrich_food_without_recipe_measures_is_per_100g(tmp_path, measures): + food = {"id": 100, "name": "Bar", "measures": measures, + "nutrients": [{"id": 208, "amount": 400}]} # fmt: skip + + entry = _enrich_one(tmp_path, food, 10, 30) + + assert entry["recipe"] is False + assert entry["nutrients"] == {208: 120.0} + + def test_enrich_recipe_entry_ignores_the_default_measure(tmp_path): entry = _enrich_one(tmp_path, LIVE_SERVING_RECIPE, 313595764, 1) diff --git a/tests/test_food_log.py b/tests/test_food_log.py index 3bfb1a8..0dd1523 100644 --- a/tests/test_food_log.py +++ b/tests/test_food_log.py @@ -370,15 +370,18 @@ def test_unknown_food_is_the_servers_not_found_error(client, tmp_path): } -def test_entry_without_numeric_grams_is_an_error_not_a_bare_entry(client): +@pytest.mark.parametrize("food_id", [1, 2]) +@pytest.mark.parametrize("grams", [None, "50", float("nan"), float("inf")]) +def test_entry_without_numeric_grams_is_an_error(client, food_id, grams): + measure_id = {1: 10, 2: 20}[food_id] client.diaries[date(2026, 10, 14)] = _diary( - _serving(7, 1, 10, None, 1), _serving(8, 2, 20, 1, 3) + _serving(7, food_id, measure_id, grams, 1), _serving(8, 2, 20, 1, 3) ) result = _call() assert result["status"] == "error" - assert "food_ids 1;" in result["message"] + assert f"Diary entry 7 ({FOODS[food_id]['name']!r}) has grams" in result["message"] def test_unknown_diary_shape_fails(client):