update_custom_food: stop trusting defaultMeasureId; require measure_id when ambiguous

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 "<amount> <name>". 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:

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 "<quantity> <name>"; 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:

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