From 02fb63645a3f8db40f04a27bbc521fbe5131142b Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Tue, 13 Oct 2026 11:51:37 -0700 Subject: [PATCH 1/3] update_custom_food: stop trusting defaultMeasureId; require measure_id when ambiguous Live probes showed defaultMeasureId is the measure logged last, not the food's serving: logging a two-measure custom food by "g" moved its default to "g". update_custom_food then edited that measure, so calories=130 meant 130 kcal per gram, and serving_grams=250 resized "g" itself, which rescaled every gram-logged diary entry (a 100 g entry read back as 10,000 g). - The serving is the food's only measure, or the new measure_id argument when it has several; ambiguity is refused, listing the measures. - A "g" or "oz" measure is never renamed or resized. - _default_measure and _only_measure are replaced by _custom_food_serving. - get_food_details documents what default_measure_id really is. Co-Authored-By: Claude Opus 5.5 --- created.json | 1 + ids.json | 1 + src/cronometer_api_mcp/client.py | 125 ++++++++++++++++++------------- src/cronometer_api_mcp/server.py | 23 ++++-- tests/test_client.py | 23 +++++- tests/test_custom_food_edit.py | 123 ++++++++++++++++++++++++++++-- 6 files changed, 233 insertions(+), 63 deletions(-) create mode 100644 created.json create mode 100644 ids.json diff --git a/created.json b/created.json new file mode 100644 index 0000000..dbb23da --- /dev/null +++ b/created.json @@ -0,0 +1 @@ +[85096125, 85096127] \ No newline at end of file diff --git a/ids.json b/ids.json new file mode 100644 index 0000000..5366a3c --- /dev/null +++ b/ids.json @@ -0,0 +1 @@ +{"A": {"food_id": 85096125, "measure_id": 313573403}, "B": {"food_id": 85096127, "measure_id": 313573407}, "gid": 313573408} \ No newline at end of file diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index 816f47f..832411d 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -84,7 +84,8 @@ # Measures every recipe carries besides its serving. Nothing but the name # marks them (the API data has no other distinguishing field). _FULL_RECIPE_MEASURE = "full recipe" -_RECIPE_FIXED_MEASURES = frozenset({"g", "oz", _FULL_RECIPE_MEASURE}) +_WEIGHT_UNIT_MEASURES = frozenset({"g", "oz"}) +_RECIPE_FIXED_MEASURES = _WEIGHT_UNIT_MEASURES | {_FULL_RECIPE_MEASURE} # Recipe serving weights are sent rounded to this many decimals, and read back # within _GRAMS_TOL: a batch that doesn't divide evenly (4561 g / 6) would # otherwise send an unbounded float. @@ -769,7 +770,7 @@ def create_custom_food( food = self.get_food(food_id) _, serving = self._settle_serving( food, - _only_measure(food)["id"], + _custom_food_serving(food, None)["id"], serving_name, serving_quantity, serving_grams, @@ -857,27 +858,6 @@ def _rename(food: dict, name: str | None) -> None: if t.get("name") == old_name: t["name"] = name - @staticmethod - def _default_measure(food: dict) -> dict | None: - """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 - 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, food_id: int, @@ -895,16 +875,28 @@ def update_custom_food( serving_name: str | None = None, serving_grams: float | None = None, serving_quantity: float | None = None, + measure_id: int | None = None, ) -> dict: """Edit a user-created custom food in place. Only the fields passed (not None) change; everything else is re-sent exactly as the server returned it. Nutrient amounts are per serving -- - the default measure's weight, or serving_grams when given -- and are + the serving measure's weight, or serving_grams when given -- and are normalized to per-100g like create_custom_food. Changing serving_grams alone leaves the stored per-100g values as they are, so the food's per-serving numbers shift with the new weight. + The serving measure is the food's only measure, or measure_id when it + has several (see _custom_food_serving). Never defaultMeasureId: the + server moves that to whichever measure was logged last (verified + live), so trusting it edited a "g" measure after a gram-logged entry. + Diary entries logged by a measure keep its count, so resizing one + rescales every past entry logged by it (verified live: after "g" was + resized to a 250 g serving, a 100 g entry logged by it read back as + 10,000 g). Hence + ambiguity is refused rather than guessed, and a "g" or "oz" measure + is never renamed or resized. + A measure is `amount` units of `name` weighing `value` grams in total, and the app renders the serving as " ". Setting the weight therefore also resets the quantity (to serving_quantity, or 1) @@ -937,32 +929,40 @@ def update_custom_food( _check_positive(label, v) _check_name(name) + per_serving = { + NUTRIENT_IDS["energy"]: calories, + NUTRIENT_IDS["protein"]: protein_g, + NUTRIENT_IDS["fat"]: fat_g, + NUTRIENT_IDS["carbs"]: carbs_g, + NUTRIENT_IDS["fiber"]: fiber_g, + NUTRIENT_IDS["sugar"]: sugar_g, + NUTRIENT_IDS["sodium"]: sodium_mg, + NUTRIENT_IDS["saturated_fat"]: saturated_fat_g, + } + per_serving = {nid: v for nid, v in per_serving.items() if v is not None} + per_serving.update(extra_nutrients or {}) + edits_measure = any( + v is not None for v in (serving_name, serving_grams, serving_quantity) + ) + edits_serving = bool(per_serving) or edits_measure + if measure_id is not None and not edits_serving: + raise ValueError( + "measure_id picks the serving that per-serving nutrients and " + "serving_* args apply to; pass it only with those" + ) + with self._food_lock(food_id): food = self._get_user_food(food_id, recipe=False) self._rename(food, name) - per_serving = { - NUTRIENT_IDS["energy"]: calories, - NUTRIENT_IDS["protein"]: protein_g, - NUTRIENT_IDS["fat"]: fat_g, - NUTRIENT_IDS["carbs"]: carbs_g, - NUTRIENT_IDS["fiber"]: fiber_g, - NUTRIENT_IDS["sugar"]: sugar_g, - NUTRIENT_IDS["sodium"]: sodium_mg, - NUTRIENT_IDS["saturated_fat"]: saturated_fat_g, - } - 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) - 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: + measure = _custom_food_serving(food, measure_id) if edits_serving else None + if edits_measure and _is_weight_unit(measure["name"]): 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" + f"Custom food {food_id}'s measure {measure['id']} is the " + f"weight unit {measure['name']!r}; renaming or resizing it " + f"would rescale every diary entry logged in it. Add a " + f"separate serving in the Cronometer app instead" ) if serving_name is not None: measure["name"] = serving_name @@ -2261,6 +2261,10 @@ def _measure_key(name: str) -> str: return name.strip().casefold() +def _is_weight_unit(name: str) -> bool: + return _measure_key(name) in _WEIGHT_UNIT_MEASURES + + def _is_fixed_recipe_measure(name: str) -> bool: return _measure_key(name) in _RECIPE_FIXED_MEASURES @@ -2384,15 +2388,36 @@ 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.""" +def _custom_food_serving(food: dict, measure_id: int | None) -> dict: + """The custom food measure that update_custom_food edits. + + measure_id when given (it must be one of the food's measures); otherwise + the food's only measure. With several and no measure_id it's ambiguous, + so that's refused: defaultMeasureId can't break the tie, since the server + sets it to the last-logged measure. + """ measures = food.get("measures") or [] - if len(measures) != 1: + label = f"Custom food {food['id']}" + if not measures: raise CronometerError( - f"Food {food['id']} has {len(measures)} measures " - f"{[m.get('name') for m in measures]}, expected exactly one" + f"{label} has no serving measure, so its serving and per-serving " + f"nutrients can't be set; fix it in the Cronometer app" ) - return measures[0] + listed = [(m["id"], m["name"], m["value"]) for m in measures] + if measure_id is None: + if len(measures) > 1: + raise CronometerError( + f"{label} has {len(measures)} measures (id, name, grams) " + f"{listed}; pass measure_id to say which serving to edit" + ) + return measures[0] + for m in measures: + if m["id"] == measure_id: + return m + raise CronometerError( + f"{label} has no measure {measure_id}; its measures (id, name, grams) " + f"are {listed}" + ) def _gram_measure_id(food: dict) -> int: diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index f7daea4..4c2bb0a 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -1095,7 +1095,9 @@ def get_food_details(food_id: int) -> str: Use this after search_foods to get the full nutrient profile and available measure_ids needed for add_food_entry. For a recipe it also returns its ingredients (in update_recipe's input shape) and comments; a - deleted (retired) food is flagged with "retired": true. + deleted (retired) food is flagged with "retired": true. default_measure_id + follows whichever measure was logged last (and may be 0), so it isn't + necessarily the food's serving. Args: food_id: Food ID from search_foods results. @@ -1235,6 +1237,7 @@ def update_custom_food( serving_name: str | None = None, serving_grams: float | None = None, serving_quantity: float | None = None, + measure_id: int | None = None, ) -> str: """Edit an existing custom food (one you created) in place. @@ -1244,10 +1247,15 @@ def update_custom_food( past days, are recalculated with the new values. Recipes cannot be edited with this tool, even though they also show as source "Custom". - Nutrient amounts are per serving: the food's default serving, or - serving_grams when you pass it. To change the serving weight without - re-entering nutrition, pass serving_grams alone; the stored per-100g - values stay put, so the per-serving numbers scale with the new weight. + Nutrient amounts are per serving: the food's serving measure, or + serving_grams when you pass it. A food with one measure needs no + measure_id; with several, pass measure_id (from get_food_details) to say + which serving the nutrients and serving_* args refer to, or the edit is + refused. To change the serving weight without re-entering nutrition, pass + serving_grams alone; the stored per-100g values stay put, so the + per-serving numbers scale with the new weight. Diary entries logged by + that measure keep their count of it, so they grow or shrink with it. A + "g" or "oz" measure can't be renamed or resized. Args: food_id: ID of the custom food to edit. @@ -1275,6 +1283,10 @@ def update_custom_food( gives "3 cookies = 30 g". To keep the per-unit weight, also pass serving_grams (45 here). Check the result with get_food_details, which lists each measure's quantity. + measure_id: The serving measure to edit, from get_food_details. + Required when the food has several measures; its + default_measure_id is just the measure logged last, so don't + assume it's the serving. """ try: client = _get_client() @@ -1293,6 +1305,7 @@ def update_custom_food( serving_name=serving_name, serving_grams=serving_grams, serving_quantity=serving_quantity, + measure_id=measure_id, ) return _ok({"food_id": result["food_id"], "name": result["name"]}) except Exception as e: diff --git a/tests/test_client.py b/tests/test_client.py index 8c3488c..bd0bfc5 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1332,10 +1332,10 @@ def test_create_custom_food_failure_after_save_names_the_id( @pytest.mark.parametrize( ("break_read_back", "match"), [ - (lambda food: food.update(measures=[]), r"has 0 measures \[\]"), + (lambda food: food.update(measures=[]), "has no serving measure"), ( lambda food: food["measures"].append({**food["measures"][0], "id": 1}), - r"has 2 measures \['serving', 'serving'\]", + r"has 2 measures \(id, name, grams\) \[\(9000, 'serving', 100", ), ], ) @@ -1417,7 +1417,10 @@ def test_update_custom_food_zero_default_with_two_measures_raises(tmp_path, kwar client, state, fake, _ = _create_gram_food(tmp_path) fake.foods[4242]["measures"].append(dict(_CUP)) - with pytest.raises(CronometerError, match=r"has 2 measures \['g', 'cup'\]"): + with pytest.raises( + CronometerError, + match=r"has 2 measures .*\(9000, 'g', 1\), \(9001, 'cup', 240\)\]; pass measure_id", + ): client.update_custom_food(4242, **kwargs) assert state["endpoints"] == ["add_food", "get_food", "get_food"] @@ -1464,3 +1467,17 @@ def test_add_custom_food_tool_sends_serving_quantity(tmp_path, monkeypatch): 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) + + +def test_update_custom_food_tool_passes_measure_id(tmp_path, monkeypatch): + client, _, fake, _ = _create_gram_food(tmp_path) + fake.foods[4242]["measures"].append(dict(_CUP)) + monkeypatch.setattr(mcp_server, "_get_client", lambda: client) + + refused = json.loads(mcp_server.update_custom_food(4242, calories=120)) + assert refused["status"] == "error" + assert "pass measure_id" in refused["message"] + + out = json.loads(mcp_server.update_custom_food(4242, calories=120, measure_id=9001)) + assert out["status"] == "success" + assert {n["id"]: n["amount"] for n in fake.foods[4242]["nutrients"]}[208] == 50.0 diff --git a/tests/test_custom_food_edit.py b/tests/test_custom_food_edit.py index e4c521f..f2f7ba2 100644 --- a/tests/test_custom_food_edit.py +++ b/tests/test_custom_food_edit.py @@ -223,7 +223,11 @@ def test_update_custom_food_serving_size_in_diary( ) client, state = make_cold_client(tmp_path, [food, OK]) client.update_custom_food( - FOOD_ID, serving_grams=30, serving_name="cookies", serving_quantity=2 + FOOD_ID, + serving_grams=30, + serving_name="cookies", + serving_quantity=2, + measure_id=900, ) saved = state["payloads"][1]["data"] @@ -572,9 +576,118 @@ def test_update_custom_food_refuses_unusable_serving_weight(tmp_path, value): 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)]) +GRAM_MEASURE = {"id": 901, "name": "g", "amount": 1, "value": 1, "type": "Weight"} - with pytest.raises(CronometerError, match="default measure 901 isn't one"): - client.update_custom_food(FOOD_ID, calories=100) + +def two_measure_food(default: int) -> dict: + """A 50 g "serving" plus a "g" measure. The server moves defaultMeasureId + to whichever measure was logged last (verified live), so after a + gram-logged entry it points at "g".""" + food = custom_food(defaultMeasureId=default) + food["measures"].append(dict(GRAM_MEASURE)) + return food + + +SERVING_EDITS = [ + {"calories": 100}, + {"extra_nutrients": {601: 5}}, + {"serving_name": "cup"}, + {"serving_grams": 30}, + {"serving_quantity": 2}, +] + + +@pytest.mark.parametrize("default", [900, 901, 0, 12345]) +@pytest.mark.parametrize("kwargs", SERVING_EDITS) +def test_update_custom_food_with_several_measures_requires_measure_id( + tmp_path, default, kwargs +): + """Whatever defaultMeasureId says, several measures make the serving + ambiguous; nothing is saved.""" + client, state = make_cold_client(tmp_path, [two_measure_food(default)]) + + with pytest.raises( + CronometerError, + match=r"has 2 measures \(id, name, grams\) \[\(900, 'serving', 50\), " + r"\(901, 'g', 1\)\]; pass measure_id", + ): + client.update_custom_food(FOOD_ID, **kwargs) + assert len(state["payloads"]) == 1 # the read, no save + + +@pytest.mark.parametrize("default", [900, 901, 0]) +def test_update_custom_food_measure_id_picks_the_serving(tmp_path, default): + """measure_id, not defaultMeasureId, decides which measure is edited and + which weight per-serving nutrients are scaled by; the other measure and + the default are left alone.""" + client, state = make_cold_client(tmp_path, [two_measure_food(default), OK]) + + client.update_custom_food(FOOD_ID, measure_id=900, calories=150, serving_grams=75) + + data = state["payloads"][1]["data"] + assert data["defaultMeasureId"] == default + serving, gram = data["measures"] + assert (serving["id"], serving["value"], serving["amount"]) == (900, 75, 1.0) + assert gram == GRAM_MEASURE + assert nutrients_by_id(state["payloads"][1])[208] == 200.0 # 150 per 75 g + + +def test_update_custom_food_measure_id_can_pick_the_gram_measure(tmp_path): + """Nutrients alone may be given per gram; the "g" measure itself stays.""" + client, state = make_cold_client(tmp_path, [two_measure_food(900), OK]) + + client.update_custom_food(FOOD_ID, measure_id=901, calories=1.3, protein_g=0.11) + + by_id = nutrients_by_id(state["payloads"][1]) + assert (by_id[208], by_id[203], by_id[-203]) == (130.0, 11.0, 11.0) + assert state["payloads"][1]["data"]["measures"][1] == GRAM_MEASURE + + +@pytest.mark.parametrize("default", [900, 0, 12345]) +def test_update_custom_food_sole_measure_ignores_default_measure_id(tmp_path, default): + """With one measure there's no ambiguity: a zero or stale defaultMeasureId + doesn't matter, and measure_id may name that measure explicitly.""" + food = custom_food(defaultMeasureId=default) + client, state = make_cold_client(tmp_path, [food, OK, food, OK]) + + client.update_custom_food(FOOD_ID, calories=100) + client.update_custom_food(FOOD_ID, calories=100, measure_id=900) + + for payload in state["payloads"][1], state["payloads"][3]: + assert nutrients_by_id(payload)[208] == 200.0 + assert payload["data"]["defaultMeasureId"] == default + + +@pytest.mark.parametrize("food", [custom_food(), two_measure_food(900)]) +def test_update_custom_food_refuses_unknown_measure_id(tmp_path, food): + client, state = make_cold_client(tmp_path, [food]) + + with pytest.raises(CronometerError, match="has no measure 777; its measures"): + client.update_custom_food(FOOD_ID, calories=100, measure_id=777) + assert len(state["payloads"]) == 1 # the read, no save + + +def test_update_custom_food_refuses_measure_id_without_serving_edit(tmp_path): + """measure_id with nothing it applies to is a caller mistake; refused + before any request.""" + client, state = make_cold_client(tmp_path, []) + + with pytest.raises(ValueError, match="pass it only with those"): + client.update_custom_food(FOOD_ID, name="Renamed", measure_id=900) + assert state["payloads"] == [] + + +@pytest.mark.parametrize("unit", ["g", "oz", " G "]) +@pytest.mark.parametrize("kwargs", SERVING_EDITS[2:]) +def test_update_custom_food_refuses_renaming_or_resizing_weight_unit( + tmp_path, unit, kwargs +): + """Resizing a measure rescales the diary entries logged by it (seen live: + after "g" became a 250 g serving, a 100 g entry read back as 10,000 g).""" + food = custom_food() + food["measures"][0].update(name=unit, value=1) + client, state = make_cold_client(tmp_path, [food]) + + with pytest.raises(CronometerError, match="is the weight unit"): + client.update_custom_food(FOOD_ID, **kwargs) assert len(state["payloads"]) == 1 # the read, no save From dea544ace7b3a90d1b59c5389cd4a651d7e99c6d Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Tue, 13 Oct 2026 11:52:29 -0700 Subject: [PATCH 2/3] Trim comments; drop probe scratch files Co-Authored-By: Claude Opus 5.5 --- created.json | 1 - ids.json | 1 - src/cronometer_api_mcp/client.py | 22 +++++----------------- src/cronometer_api_mcp/server.py | 18 ++++++------------ tests/test_custom_food_edit.py | 17 +---------------- 5 files changed, 12 insertions(+), 47 deletions(-) delete mode 100644 created.json delete mode 100644 ids.json diff --git a/created.json b/created.json deleted file mode 100644 index dbb23da..0000000 --- a/created.json +++ /dev/null @@ -1 +0,0 @@ -[85096125, 85096127] \ No newline at end of file diff --git a/ids.json b/ids.json deleted file mode 100644 index 5366a3c..0000000 --- a/ids.json +++ /dev/null @@ -1 +0,0 @@ -{"A": {"food_id": 85096125, "measure_id": 313573403}, "B": {"food_id": 85096127, "measure_id": 313573407}, "gid": 313573408} \ No newline at end of file diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index 832411d..fc15706 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -886,16 +886,9 @@ def update_custom_food( alone leaves the stored per-100g values as they are, so the food's per-serving numbers shift with the new weight. - The serving measure is the food's only measure, or measure_id when it - has several (see _custom_food_serving). Never defaultMeasureId: the - server moves that to whichever measure was logged last (verified - live), so trusting it edited a "g" measure after a gram-logged entry. - Diary entries logged by a measure keep its count, so resizing one - rescales every past entry logged by it (verified live: after "g" was - resized to a 250 g serving, a 100 g entry logged by it read back as - 10,000 g). Hence - ambiguity is refused rather than guessed, and a "g" or "oz" measure - is never renamed or resized. + Resizing a measure rescales every diary entry logged by it (seen + live), so the measure is picked by _custom_food_serving, never guessed, + and a "g" or "oz" measure is never renamed or resized. A measure is `amount` units of `name` weighing `value` grams in total, and the app renders the serving as " ". Setting the @@ -2389,13 +2382,8 @@ def _serving_fraction(batch_grams: float, servings_per_batch: float) -> float: def _custom_food_serving(food: dict, measure_id: int | None) -> dict: - """The custom food measure that update_custom_food edits. - - measure_id when given (it must be one of the food's measures); otherwise - the food's only measure. With several and no measure_id it's ambiguous, - so that's refused: defaultMeasureId can't break the tie, since the server - sets it to the last-logged measure. - """ + """measure_id's measure, else the sole one. Not defaultMeasureId: the + server moves it to the last-logged measure (seen live).""" measures = food.get("measures") or [] label = f"Custom food {food['id']}" if not measures: diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index 4c2bb0a..afb25c8 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -1096,8 +1096,7 @@ def get_food_details(food_id: int) -> str: available measure_ids needed for add_food_entry. For a recipe it also returns its ingredients (in update_recipe's input shape) and comments; a deleted (retired) food is flagged with "retired": true. default_measure_id - follows whichever measure was logged last (and may be 0), so it isn't - necessarily the food's serving. + is the measure logged last, not necessarily the serving. Args: food_id: Food ID from search_foods results. @@ -1248,14 +1247,11 @@ def update_custom_food( with this tool, even though they also show as source "Custom". Nutrient amounts are per serving: the food's serving measure, or - serving_grams when you pass it. A food with one measure needs no - measure_id; with several, pass measure_id (from get_food_details) to say - which serving the nutrients and serving_* args refer to, or the edit is - refused. To change the serving weight without re-entering nutrition, pass + serving_grams when you pass it; with several measures, measure_id picks + it. To change the serving weight without re-entering nutrition, pass serving_grams alone; the stored per-100g values stay put, so the per-serving numbers scale with the new weight. Diary entries logged by - that measure keep their count of it, so they grow or shrink with it. A - "g" or "oz" measure can't be renamed or resized. + that measure are rescaled with it, so "g" and "oz" can't be resized. Args: food_id: ID of the custom food to edit. @@ -1283,10 +1279,8 @@ def update_custom_food( gives "3 cookies = 30 g". To keep the per-unit weight, also pass serving_grams (45 here). Check the result with get_food_details, which lists each measure's quantity. - measure_id: The serving measure to edit, from get_food_details. - Required when the food has several measures; its - default_measure_id is just the measure logged last, so don't - assume it's the serving. + measure_id: The serving to edit, from get_food_details; required + when the food has several measures. """ try: client = _get_client() diff --git a/tests/test_custom_food_edit.py b/tests/test_custom_food_edit.py index f2f7ba2..678b3cf 100644 --- a/tests/test_custom_food_edit.py +++ b/tests/test_custom_food_edit.py @@ -580,9 +580,6 @@ def test_update_custom_food_refuses_unusable_serving_weight(tmp_path, value): def two_measure_food(default: int) -> dict: - """A 50 g "serving" plus a "g" measure. The server moves defaultMeasureId - to whichever measure was logged last (verified live), so after a - gram-logged entry it points at "g".""" food = custom_food(defaultMeasureId=default) food["measures"].append(dict(GRAM_MEASURE)) return food @@ -602,8 +599,6 @@ def two_measure_food(default: int) -> dict: def test_update_custom_food_with_several_measures_requires_measure_id( tmp_path, default, kwargs ): - """Whatever defaultMeasureId says, several measures make the serving - ambiguous; nothing is saved.""" client, state = make_cold_client(tmp_path, [two_measure_food(default)]) with pytest.raises( @@ -617,9 +612,6 @@ def test_update_custom_food_with_several_measures_requires_measure_id( @pytest.mark.parametrize("default", [900, 901, 0]) def test_update_custom_food_measure_id_picks_the_serving(tmp_path, default): - """measure_id, not defaultMeasureId, decides which measure is edited and - which weight per-serving nutrients are scaled by; the other measure and - the default are left alone.""" client, state = make_cold_client(tmp_path, [two_measure_food(default), OK]) client.update_custom_food(FOOD_ID, measure_id=900, calories=150, serving_grams=75) @@ -629,11 +621,10 @@ def test_update_custom_food_measure_id_picks_the_serving(tmp_path, default): serving, gram = data["measures"] assert (serving["id"], serving["value"], serving["amount"]) == (900, 75, 1.0) assert gram == GRAM_MEASURE - assert nutrients_by_id(state["payloads"][1])[208] == 200.0 # 150 per 75 g + assert nutrients_by_id(state["payloads"][1])[208] == 200.0 def test_update_custom_food_measure_id_can_pick_the_gram_measure(tmp_path): - """Nutrients alone may be given per gram; the "g" measure itself stays.""" client, state = make_cold_client(tmp_path, [two_measure_food(900), OK]) client.update_custom_food(FOOD_ID, measure_id=901, calories=1.3, protein_g=0.11) @@ -645,8 +636,6 @@ def test_update_custom_food_measure_id_can_pick_the_gram_measure(tmp_path): @pytest.mark.parametrize("default", [900, 0, 12345]) def test_update_custom_food_sole_measure_ignores_default_measure_id(tmp_path, default): - """With one measure there's no ambiguity: a zero or stale defaultMeasureId - doesn't matter, and measure_id may name that measure explicitly.""" food = custom_food(defaultMeasureId=default) client, state = make_cold_client(tmp_path, [food, OK, food, OK]) @@ -668,8 +657,6 @@ def test_update_custom_food_refuses_unknown_measure_id(tmp_path, food): def test_update_custom_food_refuses_measure_id_without_serving_edit(tmp_path): - """measure_id with nothing it applies to is a caller mistake; refused - before any request.""" client, state = make_cold_client(tmp_path, []) with pytest.raises(ValueError, match="pass it only with those"): @@ -682,8 +669,6 @@ def test_update_custom_food_refuses_measure_id_without_serving_edit(tmp_path): def test_update_custom_food_refuses_renaming_or_resizing_weight_unit( tmp_path, unit, kwargs ): - """Resizing a measure rescales the diary entries logged by it (seen live: - after "g" became a 250 g serving, a 100 g entry read back as 10,000 g).""" food = custom_food() food["measures"][0].update(name=unit, value=1) client, state = make_cold_client(tmp_path, [food]) From 29492ccdf204f7ce141ab71028e6173d92a23108 Mon Sep 17 00:00:00 2001 From: Colin Toft Date: Tue, 13 Oct 2026 11:55:11 -0700 Subject: [PATCH 3/3] Address review: refuse renaming to g/oz, drop default_measure_id output - update_custom_food refuses serving_name "g"/"oz", which would otherwise create a weight-unit measure the tool can then never fix. - get_food_details no longer returns default_measure_id: it is just the last-logged measure, so exposing it invites the same mistake. - Tool Args no longer say "default serving"; old test names renamed. Co-Authored-By: Claude Opus 5.5 --- src/cronometer_api_mcp/client.py | 5 +++++ src/cronometer_api_mcp/server.py | 10 ++++------ tests/test_client.py | 10 +++++----- tests/test_custom_food_edit.py | 9 +++++++++ 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index fc15706..d8ef4e2 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -914,6 +914,11 @@ def update_custom_food( if serving_name is not None: serving_name = _serving_name(serving_name) + if _is_weight_unit(serving_name): + raise ValueError( + f"serving_name {serving_name!r} is a weight unit; name the " + f"serving (e.g. 'cup', 'slice')" + ) for label, v in ( ("serving_grams", serving_grams), ("serving_quantity", serving_quantity), diff --git a/src/cronometer_api_mcp/server.py b/src/cronometer_api_mcp/server.py index afb25c8..03d4370 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -1095,8 +1095,7 @@ def get_food_details(food_id: int) -> str: Use this after search_foods to get the full nutrient profile and available measure_ids needed for add_food_entry. For a recipe it also returns its ingredients (in update_recipe's input shape) and comments; a - deleted (retired) food is flagged with "retired": true. default_measure_id - is the measure logged last, not necessarily the serving. + deleted (retired) food is flagged with "retired": true. Args: food_id: Food ID from search_foods results. @@ -1121,7 +1120,6 @@ def get_food_details(food_id: int) -> str: details = { "food_id": data.get("id"), "name": data.get("name"), - "default_measure_id": data.get("defaultMeasureId"), "measures": measures, "nutrients": data.get("nutrients", []), } @@ -1267,12 +1265,12 @@ def update_custom_food( extra_nutrients: Additional nutrients keyed by Cronometer nutrient ID (from get_daily_nutrition) and valued per serving; must not reuse an ID the named args already cover. - serving_name: New name for the default serving. The app shows the + serving_name: New name for the serving; not "g" or "oz". The app shows the serving as " "; a leading number ("2 cookies") is refused: pass it as serving_quantity. - serving_grams: New total weight of the default serving in grams. + serving_grams: New total weight of the serving in grams. Also resets the quantity to serving_quantity (or 1). - serving_quantity: How many units make up the default serving + serving_quantity: How many units make up the serving (default 1 whenever serving_grams is passed). Without serving_grams the serving's total weight stays the same, so each unit gets lighter or heavier: changing "2 cookies = 30 g" to 3 diff --git a/tests/test_client.py b/tests/test_client.py index bd0bfc5..2ffd6f5 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1394,7 +1394,7 @@ def test_create_gram_custom_food_with_zero_default_measure(tmp_path): assert state["endpoints"] == ["add_food", "get_food"] -def test_update_custom_food_with_zero_default_measure(tmp_path): +def test_update_custom_food_gram_food_takes_per_gram_nutrients(tmp_path): client, state, fake, _ = _create_gram_food(tmp_path) client.update_custom_food(4242, name="Renamed", calories=3) @@ -1413,7 +1413,7 @@ def test_update_custom_food_with_zero_default_measure(tmp_path): @pytest.mark.parametrize("kwargs", [{"calories": 3}, {"serving_grams": 2}]) -def test_update_custom_food_zero_default_with_two_measures_raises(tmp_path, kwargs): +def test_update_custom_food_two_measures_need_measure_id(tmp_path, kwargs): client, state, fake, _ = _create_gram_food(tmp_path) fake.foods[4242]["measures"].append(dict(_CUP)) @@ -1425,8 +1425,8 @@ def test_update_custom_food_zero_default_with_two_measures_raises(tmp_path, kwar 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.""" +def test_update_custom_food_two_measures_still_rename(tmp_path): + """A rename never touches a measure, so an ambiguous serving doesn't matter.""" client, state, fake, _ = _create_gram_food(tmp_path) fake.foods[4242]["measures"].append(dict(_CUP)) @@ -1436,7 +1436,7 @@ def test_update_custom_food_zero_default_with_two_measures_still_renames(tmp_pat assert fake.foods[4242]["name"] == "Renamed" -def test_update_custom_food_zero_default_with_lone_non_gram_measure(tmp_path): +def test_update_custom_food_lone_measure_is_the_serving(tmp_path): client, state, fake, _ = _create_gram_food(tmp_path) fake.foods[4242]["measures"] = [dict(_CUP)] diff --git a/tests/test_custom_food_edit.py b/tests/test_custom_food_edit.py index 678b3cf..edd3097 100644 --- a/tests/test_custom_food_edit.py +++ b/tests/test_custom_food_edit.py @@ -676,3 +676,12 @@ def test_update_custom_food_refuses_renaming_or_resizing_weight_unit( with pytest.raises(CronometerError, match="is the weight unit"): client.update_custom_food(FOOD_ID, **kwargs) assert len(state["payloads"]) == 1 # the read, no save + + +@pytest.mark.parametrize("unit", ["g", "oz", " G "]) +def test_update_custom_food_refuses_renaming_serving_to_weight_unit(tmp_path, unit): + client, state = make_cold_client(tmp_path, []) + + with pytest.raises(ValueError, match="is a weight unit"): + client.update_custom_food(FOOD_ID, serving_name=unit) + assert state["payloads"] == []