Skip to content
This repository was archived by the owner on Apr 30, 2026. It is now read-only.

Commit 8109aba

Browse files
a-dealclaude
andcommitted
Fix evening scheduler: NULL strength data crashed all user check-ins
Bodyweight exercises (sled pull, reverse hyper, etc.) logged with NULL weight_lbs in strength_set. db_read.get_strength() converted str(None) to "None", then strength.py float("None") crashed. The single try/except in _gather_context meant the crash in _checkin discarded all context, so has_composable_data() returned False and every user got the onboarding "no health data" message. Root cause: db_read.py str(None) -> "None" for NULL SQLite values. Structural cause: _gather_context wrapped all data sources in one try block. Fixes: - db_read.get_strength: NULL weight/reps/rpe -> "0"/"" not "None" - db_read.get_weights/get_bp: filter out NULL rows (defense-in-depth) - strength.py: defensive float() with try/except - scoring engine: _apply_clinical catches bad float values - briefing.py: _load_weight_log/_load_bp_log skip bad rows - scheduler.py: separate try/except per data source in _gather_context Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 296c618 commit 8109aba

6 files changed

Lines changed: 145 additions & 17 deletions

File tree

engine/coaching/briefing.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,7 +1059,13 @@ def _load_weight_log(data_dir: Path, person_id: str | None = None) -> Optional[l
10591059
user_id = _user_id_from_person(person_id)
10601060
rows = get_weights(user_id, data_dir)
10611061
if rows:
1062-
return [{"weight": float(r["weight_lbs"]), "date": r["date"]} for r in rows]
1062+
result = []
1063+
for r in rows:
1064+
try:
1065+
result.append({"weight": float(r["weight_lbs"]), "date": r["date"]})
1066+
except (ValueError, TypeError):
1067+
continue
1068+
return result or None
10631069
return None
10641070

10651071

@@ -1068,7 +1074,13 @@ def _load_bp_log(data_dir: Path, person_id: str | None = None) -> Optional[list]
10681074
user_id = _user_id_from_person(person_id)
10691075
rows = get_bp(user_id, data_dir)
10701076
if rows:
1071-
return [{"sys": float(r["systolic"]), "dia": float(r["diastolic"]), "date": r.get("date", "")} for r in rows]
1077+
result = []
1078+
for r in rows:
1079+
try:
1080+
result.append({"sys": float(r["systolic"]), "dia": float(r["diastolic"]), "date": r.get("date", "")})
1081+
except (ValueError, TypeError):
1082+
continue
1083+
return result or None
10721084
return None
10731085

10741086

engine/db_read.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ def get_weights(user_id: str | None = None, data_dir: Path | None = None) -> lis
273273
).fetchall()
274274
db.close()
275275
if rows:
276-
return [{"date": r["date"], "weight_lbs": str(r["weight_lbs"]), "source": r["source"] or ""} for r in rows]
276+
return [{"date": r["date"], "weight_lbs": str(r["weight_lbs"]), "source": r["source"] or ""} for r in rows if r["weight_lbs"] is not None]
277277
except Exception:
278278
pass
279279
# CSV fallback (repo mode only)
@@ -296,7 +296,7 @@ def get_bp(user_id: str | None = None, data_dir: Path | None = None) -> list[dic
296296
).fetchall()
297297
db.close()
298298
if rows:
299-
return [{"date": r["date"], "systolic": str(r["systolic"]), "diastolic": str(r["diastolic"]), "source": r["source"] or ""} for r in rows]
299+
return [{"date": r["date"], "systolic": str(r["systolic"]), "diastolic": str(r["diastolic"]), "source": r["source"] or ""} for r in rows if r["systolic"] is not None and r["diastolic"] is not None]
300300
except Exception:
301301
pass
302302
if data_dir:
@@ -471,7 +471,10 @@ def get_strength(user_id: str | None = None, data_dir: Path | None = None) -> li
471471
).fetchall()
472472
db.close()
473473
if rows:
474-
return [{"date": r["date"], "exercise": r["exercise"], "weight_lbs": str(r["weight_lbs"]), "reps": str(r["reps"]), "rpe": str(r["rpe"] or "")} for r in rows]
474+
return [{"date": r["date"], "exercise": r["exercise"],
475+
"weight_lbs": str(r["weight_lbs"]) if r["weight_lbs"] is not None else "0",
476+
"reps": str(r["reps"]) if r["reps"] is not None else "0",
477+
"rpe": str(r["rpe"]) if r["rpe"] is not None else ""} for r in rows]
475478
except Exception:
476479
pass
477480
if data_dir:

engine/gateway/scheduler.py

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -595,17 +595,27 @@ def _gather_context(schedule_type: str, user_id: str) -> dict:
595595
from mcp_server.tools import _checkin, _score, _get_protocols
596596

597597
context = {}
598-
try:
599-
if schedule_type in ("morning_brief", "evening_checkin"):
598+
599+
# Each data source gets its own try/except so a failure in one
600+
# (e.g. _get_protocols) doesn't discard already-fetched data (e.g. checkin).
601+
if schedule_type in ("morning_brief", "evening_checkin", "weekly_review"):
602+
try:
600603
context["checkin"] = _checkin(user_id=user_id)
601-
if schedule_type == "evening_checkin":
604+
except Exception as e:
605+
logger.warning("Failed to gather checkin for %s/%s: %s", user_id, schedule_type, e)
606+
context["checkin_error"] = str(e)
607+
608+
if schedule_type == "evening_checkin":
609+
try:
602610
context["protocols"] = _get_protocols(user_id=user_id)
603-
if schedule_type == "weekly_review":
604-
context["checkin"] = _checkin(user_id=user_id)
611+
except Exception as e:
612+
logger.warning("Failed to gather protocols for %s/%s: %s", user_id, schedule_type, e)
613+
614+
if schedule_type == "weekly_review":
615+
try:
605616
context["score"] = _score(user_id=user_id)
606-
except Exception as e:
607-
logger.warning("Failed to gather context for %s/%s: %s", user_id, schedule_type, e)
608-
context["error"] = str(e)
617+
except Exception as e:
618+
logger.warning("Failed to gather score for %s/%s: %s", user_id, schedule_type, e)
609619

610620
return context
611621

engine/scoring/engine.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,11 @@ def assess(value: Optional[float], table: dict, demo: Demographics,
148148
def _apply_clinical(result: MetricResult, clinical_key: str, value, demo: Demographics):
149149
"""Apply clinical zone assessment to a MetricResult."""
150150
if value is not None:
151-
zone, note = clinical_assess(clinical_key, float(value), demo.age, demo.sex)
151+
try:
152+
fval = float(value)
153+
except (ValueError, TypeError):
154+
return
155+
zone, note = clinical_assess(clinical_key, fval, demo.age, demo.sex)
152156
result.clinical_zone = zone
153157
result.clinical_note = note
154158

engine/tracking/strength.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,20 @@ def progression_summary(
8282

8383
# Calculate estimated 1RM for each set
8484
for s in filtered:
85-
weight = float(s.get("weight_lbs", 0))
86-
reps = int(s.get("reps", 0) or 0)
87-
rpe = float(s["rpe"]) if s.get("rpe") else None
85+
try:
86+
weight = float(s.get("weight_lbs") or 0)
87+
except (ValueError, TypeError):
88+
weight = 0
89+
try:
90+
reps = int(float(s.get("reps") or 0))
91+
except (ValueError, TypeError):
92+
reps = 0
93+
rpe = None
94+
if s.get("rpe"):
95+
try:
96+
rpe = float(s["rpe"])
97+
except (ValueError, TypeError):
98+
pass
8899
if weight > 0 and reps > 0:
89100
s["est_1rm"] = est_1rm(weight, reps, rpe)
90101
else:

tests/test_scheduler.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,3 +725,91 @@ def test_standalone_endpoint_calls_measure(self, mock_measure):
725725
data = resp.json()
726726
assert data["measured_count"] == 1
727727
mock_measure.assert_called_once()
728+
729+
730+
# --- _gather_context isolation tests ---
731+
732+
733+
class TestGatherContextIsolation:
734+
"""Verify that _gather_context uses separate try/except blocks so a
735+
failure in one data source doesn't discard already-fetched data."""
736+
737+
def test_checkin_failure_sets_error_key(self):
738+
"""If _checkin throws, context should have checkin_error but no checkin."""
739+
from engine.gateway.scheduler import _gather_context
740+
741+
with patch("mcp_server.tools._checkin", side_effect=ValueError("could not convert string to float: 'None'")):
742+
ctx = _gather_context("morning_brief", "andrew")
743+
744+
assert "checkin" not in ctx
745+
assert "checkin_error" in ctx
746+
assert "'None'" in ctx["checkin_error"]
747+
748+
def test_protocols_failure_preserves_checkin(self):
749+
"""If _get_protocols throws, the already-fetched checkin should survive."""
750+
from engine.gateway.scheduler import _gather_context
751+
752+
fake_checkin = {"data_available": {"garmin": True}, "score": {"coverage": 6}}
753+
with patch("mcp_server.tools._checkin", return_value=fake_checkin), \
754+
patch("mcp_server.tools._get_protocols", side_effect=Exception("db locked")):
755+
ctx = _gather_context("evening_checkin", "andrew")
756+
757+
assert ctx["checkin"] == fake_checkin
758+
assert "protocols" not in ctx
759+
760+
def test_score_failure_preserves_checkin_for_weekly(self):
761+
"""If _score throws during weekly_review, checkin should survive."""
762+
from engine.gateway.scheduler import _gather_context
763+
764+
fake_checkin = {"data_available": {"garmin": True}, "score": {"coverage": 6}}
765+
with patch("mcp_server.tools._checkin", return_value=fake_checkin), \
766+
patch("mcp_server.tools._score", side_effect=Exception("timeout")):
767+
ctx = _gather_context("weekly_review", "andrew")
768+
769+
assert ctx["checkin"] == fake_checkin
770+
assert "score" not in ctx
771+
772+
773+
# --- db_read NULL handling tests ---
774+
775+
776+
class TestStrengthNoneHandling:
777+
"""Verify that strength.py handles 'None' string values from db_read
778+
without crashing. This was the root cause of the evening scheduler bug:
779+
bodyweight exercises have NULL weight_lbs in SQLite, db_read.get_strength
780+
converted str(None) -> 'None', then float('None') crashed."""
781+
782+
def test_progression_handles_none_string_weight(self):
783+
"""progression_summary should not crash on 'None' weight_lbs."""
784+
from engine.tracking.strength import progression_summary
785+
786+
lift_history = [
787+
{"date": "2026-04-04", "exercise": "Squat", "weight_lbs": "315", "reps": "5", "rpe": "8"},
788+
{"date": "2026-04-04", "exercise": "Squat", "weight_lbs": "None", "reps": "None", "rpe": ""},
789+
{"date": "2026-04-04", "exercise": "Squat", "weight_lbs": "0", "reps": "0", "rpe": ""},
790+
]
791+
result = progression_summary(lift_history, "Squat")
792+
assert result is not None
793+
assert result["peak_1rm"] > 0
794+
795+
def test_progression_handles_none_reps(self):
796+
"""progression_summary should not crash on 'None' reps."""
797+
from engine.tracking.strength import progression_summary
798+
799+
lift_history = [
800+
{"date": "2026-04-04", "exercise": "Sled Pull", "weight_lbs": "None", "reps": "None", "rpe": "None"},
801+
]
802+
result = progression_summary(lift_history, "Sled Pull")
803+
assert result is not None
804+
805+
def test_clinical_assess_handles_none_string(self):
806+
"""_apply_clinical should not crash on string 'None' values."""
807+
from engine.scoring.engine import _apply_clinical
808+
from engine.models import Demographics, MetricResult
809+
810+
demo = Demographics(age=35, sex="M")
811+
result = MetricResult(name="rhr", tier=1, rank=1, has_data=True)
812+
# Should not raise - the string "None" should be caught
813+
_apply_clinical(result, "rhr", "None", demo)
814+
# clinical_zone should remain at its default (empty string), not crash
815+
assert result.clinical_zone == ""

0 commit comments

Comments
 (0)