From f1ac56609e5aa5508a7281038888a7df352ec738 Mon Sep 17 00:00:00 2001 From: Chris Farhood Date: Mon, 20 Jul 2026 09:51:39 -0400 Subject: [PATCH] fix(wellness): harden Form/TSB and date rendering from code review Address findings from the pre-merge review: - Form (TSB) now computes only when ctl/atl are numeric (isinstance guard), so a non-numeric value can no longer raise out of format_wellness_entry and take down the entire wellness render. - The Date line uses `or` chaining so a present-but-null `date` falls back to `id` instead of rendering "Date: None". - update_wellness injects the written date into the API echo when it lacks id/date, so the confirmation body can't read "Date: N/A" under a dated header. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01NGzHtDvJur9U7ysgRKRUTN --- src/intervals_mcp_server/tools/wellness.py | 4 ++++ src/intervals_mcp_server/utils/formatting.py | 12 ++++++++---- tests/test_formatting.py | 14 ++++++++++++++ tests/test_wellness.py | 8 ++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/intervals_mcp_server/tools/wellness.py b/src/intervals_mcp_server/tools/wellness.py index e1e023f..bd2199f 100644 --- a/src/intervals_mcp_server/tools/wellness.py +++ b/src/intervals_mcp_server/tools/wellness.py @@ -180,6 +180,10 @@ async def update_wellness( # pylint: disable=too-many-arguments,too-many-positi return f"Error updating wellness data: {result.get('message')}" # Intervals.icu echoes back the full updated record; render it for confirmation. + # If the echo omits the date, inject the one we wrote to so the confirmation + # body doesn't read "Date: N/A" under a dated header. if isinstance(result, dict): + if not result.get("id") and not result.get("date"): + result["date"] = date return f"Updated wellness for {date}:\n\n" + format_wellness_entry(result) return f"Updated wellness for {date}." diff --git a/src/intervals_mcp_server/utils/formatting.py b/src/intervals_mcp_server/utils/formatting.py index 2fa6146..3e93efe 100644 --- a/src/intervals_mcp_server/utils/formatting.py +++ b/src/intervals_mcp_server/utils/formatting.py @@ -167,9 +167,11 @@ def _format_training_metrics(entries: dict[str, Any]) -> list[str]: # Form (a.k.a. TSB, Training Stress Balance) = CTL - ATL. Intervals.icu does # not return this on the wellness record, so compute it when both components - # are present. Positive = fresher/tapered, negative = carrying fatigue. + # are present AND numeric. The isinstance guard keeps a non-numeric value from + # raising out of the formatter and taking down the whole wellness render. + # Positive = fresher/tapered, negative = carrying fatigue. ctl, atl = entries.get("ctl"), entries.get("atl") - if ctl is not None and atl is not None: + if isinstance(ctl, (int, float)) and isinstance(atl, (int, float)): training_metrics.append(f"- Form (TSB): {ctl - atl:.1f}") for k, label in [ @@ -350,8 +352,10 @@ def format_wellness_entry(entries: dict[str, Any], include_all_fields: bool = Fa lines = ["Wellness Data:"] # The wellness record's own date lives in `id` (e.g. "2025-05-24"); some call - # sites also inject an explicit `date`. Prefer `date`, fall back to `id`. - lines.append(f"Date: {entries.get('date', entries.get('id', 'N/A'))}") + # sites also inject an explicit `date`. Prefer `date`, fall back to `id`. Use + # `or` chaining (not get-with-default) so a present-but-null `date` still falls + # back rather than rendering "Date: None". + lines.append(f"Date: {entries.get('date') or entries.get('id') or 'N/A'}") lines.append("") training_metrics = _format_training_metrics(entries) diff --git a/tests/test_formatting.py b/tests/test_formatting.py index 9d92d3b..0a6749b 100644 --- a/tests/test_formatting.py +++ b/tests/test_formatting.py @@ -82,6 +82,20 @@ def test_format_wellness_entry_prefers_explicit_date_over_id(): assert "Date: 2024-06-02" in result +def test_format_wellness_entry_null_date_falls_back_to_id(): + """A present-but-null `date` falls back to `id`, not 'None'.""" + result = format_wellness_entry({"id": "2024-06-01", "date": None, "ctl": 50}) + assert "Date: 2024-06-01" in result + assert "Date: None" not in result + + +def test_format_wellness_entry_non_numeric_ctl_atl_does_not_crash(): + """Non-numeric ctl/atl must not raise; Form is simply omitted.""" + result = format_wellness_entry({"id": "2024-06-01", "ctl": "n/a", "atl": "n/a"}) + assert "Form (TSB)" not in result + assert "Date: 2024-06-01" in result + + def test_format_wellness_entry_include_all_fields(): """ Test that format_wellness_entry with include_all_fields=True includes additional unknown fields. diff --git a/tests/test_wellness.py b/tests/test_wellness.py index d437965..398fb06 100644 --- a/tests/test_wellness.py +++ b/tests/test_wellness.py @@ -126,3 +126,11 @@ def test_update_wellness_non_dict_result(monkeypatch): _patch_request(monkeypatch, []) out = asyncio.run(wellness.update_wellness(date="2025-05-24", weight=80)) assert out == "Updated wellness for 2025-05-24." + + +def test_update_wellness_echo_without_date_shows_written_date(monkeypatch): + # If the API echo omits id/date, the confirmation body must not read "Date: N/A". + _patch_request(monkeypatch, {"weight": 80}) + out = asyncio.run(wellness.update_wellness(date="2025-05-24", weight=80)) + assert "Date: 2025-05-24" in out + assert "Date: N/A" not in out