diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index 816f47f..d8ef4e2 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,21 @@ 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. + 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 weight therefore also resets the quantity (to serving_quantity, or 1) @@ -929,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), @@ -937,32 +927,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 +2259,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 +2386,31 @@ 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: + """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 [] - 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..03d4370 100644 --- a/src/cronometer_api_mcp/server.py +++ b/src/cronometer_api_mcp/server.py @@ -1120,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", []), } @@ -1235,6 +1234,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 +1244,12 @@ 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; 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 are rescaled with it, so "g" and "oz" can't be resized. Args: food_id: ID of the custom food to edit. @@ -1263,18 +1265,20 @@ 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 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 to edit, from get_food_details; required + when the food has several measures. """ try: client = _get_client() @@ -1293,6 +1297,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..2ffd6f5 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", ), ], ) @@ -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,17 +1413,20 @@ 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)) - 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"] -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)) @@ -1433,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)] @@ -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..edd3097 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,112 @@ 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: + 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 +): + 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): + 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 + + +def test_update_custom_food_measure_id_can_pick_the_gram_measure(tmp_path): + 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): + 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): + 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 +): + 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 + + +@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"] == []