Skip to content

Commit 7d0154b

Browse files
fdsimoes-gitclaude
andcommitted
fix(llm): normalize SDK content blocks + route through db.local_today
Two issues from the new Copilot review pass. 1) chat() was appending response.content (real Anthropic SDK pydantic blocks) verbatim to messages on tool_use iterations. The SDK *does* accept its own block types on input, but normalizing to plain dicts is more robust across SDK versions and makes the wire format deterministic. New _block_to_dict helper: - dicts pass through unchanged - SDK objects use .model_dump() with try/except fallback - SimpleNamespace / arbitrary attrs fall back to manual extraction for tool_use / text shapes The chat loop now serializes content via [_block_to_dict(b) for b in response.content] before re-posting. 2) Promoted db._local_today → db.local_today (single underscore-free public name) and routed _execute_chat_tool's two day-bucket lookups through it. Now the patch hook used by the test_nutrition.py regression cases (test_recent_calorie_balance_anchors_on_local_today, etc.) covers the chat tool dispatcher too — one patchable seam for all "what local day is it?" computations across db.py and llm.py. Tests (+4) - test_chat_normalizes_content_blocks_to_dicts_before_re_sending: asserts the assistant turn that goes back to messages.create has plain dicts (not SimpleNamespace / SDK objects) and the round-trip preserves type/id/name/input. - test_block_to_dict_uses_model_dump_when_available: real-SDK path. - test_block_to_dict_falls_back_when_model_dump_raises: defensive path. - test_execute_chat_tool_uses_db_local_today: patches db.local_today to a fixed date and confirms get_balance resolves "today" through it. Tests adjusted (rename _local_today → local_today across): - src/db.py (definition + 4 callsites) - tests/test_nutrition.py (4 patches + sanity test) - tests/test_training_intel.py (1 doc reference) Test count: 144 → 148. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 99d722b commit 7d0154b

5 files changed

Lines changed: 130 additions & 21 deletions

File tree

src/db.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@
7373
"""
7474

7575

76-
def _local_today() -> date:
76+
def local_today() -> date:
7777
"""The Pi's local 'today'.
7878
7979
The codebase has a deliberate split: timestamps are stored in UTC (they
@@ -181,7 +181,7 @@ def recent_activities(db_path: Path, days: int = 7) -> list[dict]:
181181
"""Return activities with `date` within the last `days` days, newest first."""
182182
# `activities.date` is local-formatted (Garmin's startTimeLocal[:10]),
183183
# so the cutoff must be Pi-local — not UTC.
184-
cutoff = (_local_today() - timedelta(days=days)).isoformat()
184+
cutoff = (local_today() - timedelta(days=days)).isoformat()
185185
with connect(db_path) as conn:
186186
conn.row_factory = sqlite3.Row
187187
rows = conn.execute(
@@ -439,7 +439,7 @@ def prune_old_data(con: sqlite3.Connection, days: int = 90) -> dict[str, int]:
439439
# `meal_time` is stored as a UTC ISO timestamp → cutoff must be UTC.
440440
# `activities.date` is local-formatted → cutoff must be Pi-local.
441441
cutoff_ts = (datetime.now(timezone.utc) - timedelta(days=days)).isoformat(timespec="seconds")
442-
cutoff_date = (_local_today() - timedelta(days=days)).isoformat()
442+
cutoff_date = (local_today() - timedelta(days=days)).isoformat()
443443
deleted: dict[str, int] = {}
444444
cur = con.execute("DELETE FROM activities WHERE date < ?", (cutoff_date,))
445445
deleted["activities"] = cur.rowcount or 0
@@ -573,9 +573,9 @@ def recent_calorie_balance(db_path: Path, days: int, cfg) -> list[dict]:
573573
574574
Pulls the per-day balance via `calorie_balance_for_date` so the math stays
575575
consistent with the CLI / dashboard output. Anchored on Pi-local "today"
576-
to match the rest of the codebase (see `_local_today`).
576+
to match the rest of the codebase (see `local_today`).
577577
"""
578-
today = _local_today()
578+
today = local_today()
579579
out: list[dict] = []
580580
for offset in range(days - 1, -1, -1):
581581
d = (today - timedelta(days=offset)).isoformat()
@@ -687,9 +687,9 @@ def daily_readiness_history(db_path: Path, days: int, cfg) -> list[dict]:
687687
688688
Skips days where the score is None (no data) — caller renders empty cells.
689689
Anchored on Pi-local "today" so the heatmap's most recent cell stays
690-
aligned with what the user calls "today" (see `_local_today`).
690+
aligned with what the user calls "today" (see `local_today`).
691691
"""
692-
today = _local_today()
692+
today = local_today()
693693
out: list[dict] = []
694694
for offset in range(days - 1, -1, -1):
695695
d = (today - timedelta(days=offset)).isoformat()

src/llm.py

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,38 @@ def _block_input(block) -> dict:
231231
return dict((block or {}).get("input") or {})
232232

233233

234+
def _block_to_dict(block) -> dict:
235+
"""Normalize a content block (SDK pydantic, plain dict, or test
236+
SimpleNamespace) to a JSON-serializable dict suitable for sending back to
237+
the API on the next loop iteration. The Anthropic SDK *does* accept its
238+
own block objects on input, but normalizing keeps the wire format stable
239+
and avoids any reliance on SDK serializer internals.
240+
"""
241+
if isinstance(block, dict):
242+
return dict(block)
243+
# Real SDK objects expose `model_dump`.
244+
if hasattr(block, "model_dump"):
245+
try:
246+
return block.model_dump()
247+
except Exception: # noqa: BLE001 — fall through to manual extraction
248+
pass
249+
# Manual fallback (covers SimpleNamespace and any future block shape).
250+
btype = getattr(block, "type", None)
251+
if btype == "tool_use":
252+
return {
253+
"type": "tool_use",
254+
"id": getattr(block, "id", None),
255+
"name": getattr(block, "name", None),
256+
"input": getattr(block, "input", {}) or {},
257+
}
258+
if btype == "text":
259+
return {"type": "text", "text": getattr(block, "text", "")}
260+
# Last-resort: copy public attrs into a dict.
261+
return {"type": btype, **{
262+
k: v for k, v in vars(block).items() if not k.startswith("_")
263+
}}
264+
265+
234266
def extract_meal_from_text(cfg: Config, user_text: str) -> dict | None:
235267
"""Run a meal description through Claude and return the structured meal dict.
236268
@@ -553,7 +585,11 @@ def _execute_chat_tool(
553585
"""
554586
if pending is None:
555587
pending = {}
556-
today = date.today().isoformat()
588+
# Route every "what local day is it?" lookup through `db.local_today` so
589+
# the chat tools share the codebase-wide patchable seam (mocked in the
590+
# _local_today regression tests in test_nutrition.py).
591+
today_d = db.local_today()
592+
today = today_d.isoformat()
557593
target = (input_data.get("date") or today)
558594

559595
# ── Read tools ────────────────────────────────────────────────────────
@@ -564,7 +600,6 @@ def _execute_chat_tool(
564600
if name == "get_recent_meals":
565601
days = max(1, min(int(input_data.get("days") or 7), 30))
566602
out: list[dict] = []
567-
today_d = date.today()
568603
for offset in range(days - 1, -1, -1):
569604
d_iso = (today_d - timedelta(days=offset)).isoformat()
570605
out.extend(db.meals_for_date(cfg.db_path, d_iso))
@@ -783,7 +818,14 @@ def chat(
783818
)
784819

785820
if getattr(response, "stop_reason", None) == "tool_use":
786-
messages.append({"role": "assistant", "content": list(response.content)})
821+
# Normalize SDK content blocks to plain dicts before re-sending.
822+
# The SDK accepts its own pydantic blocks on input, but plain
823+
# dicts are more robust across SDK versions and let our test
824+
# fixtures (SimpleNamespace) round-trip through the same path.
825+
messages.append({
826+
"role": "assistant",
827+
"content": [_block_to_dict(b) for b in response.content],
828+
})
787829
tool_results: list[dict] = []
788830
for block in response.content:
789831
btype = getattr(block, "type", None) or (

tests/test_llm.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,73 @@ def test_chat_with_history_and_image_sends_image_only_for_current_turn():
624624
assert any(c.get("type") == "image" for c in sent[-1]["content"])
625625

626626

627+
def test_chat_normalizes_content_blocks_to_dicts_before_re_sending():
628+
"""The assistant turn appended to messages must contain plain dicts, not
629+
SDK objects — protects against pydantic-vs-dict serializer mismatches on
630+
the second loop iteration."""
631+
cfg = _make_cfg(anthropic_api_key="sk-ant-api03-x")
632+
tool_use_resp = SimpleNamespace(
633+
content=[_tool_use_block("tu_1", "get_balance", {})],
634+
stop_reason="tool_use",
635+
)
636+
final_resp = SimpleNamespace(
637+
content=[_text_block("done")], stop_reason="end_turn",
638+
)
639+
fake_client = MagicMock()
640+
fake_client.messages.create.side_effect = [tool_use_resp, final_resp]
641+
with patch("src.llm.build_anthropic_client", return_value=fake_client), \
642+
patch("src.llm.db.calorie_balance_for_date", return_value={"balance_kcal": 0}):
643+
llm.chat(cfg, "x", {})
644+
# Second call's messages should include the assistant turn with content
645+
# as a list of plain dicts (not SimpleNamespace / SDK objects).
646+
second = fake_client.messages.create.call_args_list[1].kwargs["messages"]
647+
assistant_turn = next(m for m in second if m["role"] == "assistant")
648+
for block in assistant_turn["content"]:
649+
assert isinstance(block, dict), f"block is {type(block)}, expected dict"
650+
assert "type" in block
651+
# And it round-trips losslessly: still a tool_use with the same id+name+input.
652+
tool_block = assistant_turn["content"][0]
653+
assert tool_block["type"] == "tool_use"
654+
assert tool_block["id"] == "tu_1"
655+
assert tool_block["name"] == "get_balance"
656+
657+
658+
def test_block_to_dict_uses_model_dump_when_available():
659+
"""Real SDK blocks expose .model_dump(); use that path."""
660+
class FakeSDKBlock:
661+
def model_dump(self):
662+
return {"type": "tool_use", "id": "tu_x", "name": "y", "input": {"a": 1}}
663+
664+
out = llm._block_to_dict(FakeSDKBlock())
665+
assert out == {"type": "tool_use", "id": "tu_x", "name": "y", "input": {"a": 1}}
666+
667+
668+
def test_block_to_dict_falls_back_when_model_dump_raises():
669+
"""If model_dump raises (corrupt SDK state), the manual SimpleNamespace
670+
fallback still produces a usable dict."""
671+
class BrokenSDKBlock:
672+
type = "text"
673+
text = "hello"
674+
def model_dump(self):
675+
raise RuntimeError("boom")
676+
677+
out = llm._block_to_dict(BrokenSDKBlock())
678+
assert out["type"] == "text"
679+
assert out["text"] == "hello"
680+
681+
682+
def test_execute_chat_tool_uses_db_local_today(tmpdb_cfg):
683+
"""The dispatcher must route every 'what local day is it?' lookup through
684+
db.local_today so day-bucketing is patchable in one place."""
685+
from datetime import date as _date
686+
fake_today = _date(2026, 5, 5)
687+
with patch("src.llm.db.local_today", return_value=fake_today) as mock_local:
688+
# get_balance with no date input → should resolve to local_today's ISO.
689+
out = llm._execute_chat_tool("get_balance", {}, tmpdb_cfg, {})
690+
assert mock_local.called
691+
assert out["date"] == "2026-05-05"
692+
693+
627694
def test_chat_invokes_progress_cb_once_per_tool_use_block():
628695
"""The progress callback fires before each tool's dispatcher runs and is
629696
given the tool name + input dict — never the SDK block itself."""

tests/test_nutrition.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ def test_meal_timing_summary_computes_window_and_count(tmpdb):
153153

154154
def test_recent_calorie_balance_returns_one_row_per_day(tmpdb):
155155
cfg = _make_cfg(tmpdb)
156-
# Production anchors on Pi-local 'today' (db._local_today). Match here
156+
# Production anchors on Pi-local 'today' (db.local_today). Match here
157157
# so this test isn't UTC-flaky around midnight.
158158
today = date.today()
159159
db.insert_meal(tmpdb, {"description": "today's lunch", "source": "manual", "kcal": 600.0})
@@ -221,32 +221,32 @@ def test_delete_meal_returns_false_for_missing(tmpdb):
221221
assert db.delete_meal(tmpdb, 99999) is False
222222

223223

224-
def test_local_today_helper_returns_local_date():
225-
"""Sanity: _local_today() agrees with date.today() right now."""
226-
assert db._local_today() == date.today()
224+
def testlocal_today_helper_returns_local_date():
225+
"""Sanity: local_today() agrees with date.today() right now."""
226+
assert db.local_today() == date.today()
227227

228228

229-
def test_recent_calorie_balance_anchors_on_local_today(tmpdb):
229+
def test_recent_calorie_balance_anchors_onlocal_today(tmpdb):
230230
"""Last entry in the 7-day balance series must be local today, not UTC."""
231231
from unittest.mock import patch
232232
cfg = _make_cfg(tmpdb)
233233
fake_today = date(2026, 5, 5) # local
234-
with patch("src.db._local_today", return_value=fake_today):
234+
with patch("src.db.local_today", return_value=fake_today):
235235
out = db.recent_calorie_balance(tmpdb, days=7, cfg=cfg)
236236
assert out[-1]["date"] == "2026-05-05"
237237
assert out[0]["date"] == "2026-04-29"
238238

239239

240-
def test_daily_readiness_history_anchors_on_local_today(tmpdb):
240+
def test_daily_readiness_history_anchors_onlocal_today(tmpdb):
241241
from unittest.mock import patch
242242
cfg = _make_cfg(tmpdb)
243243
fake_today = date(2026, 5, 5)
244-
with patch("src.db._local_today", return_value=fake_today):
244+
with patch("src.db.local_today", return_value=fake_today):
245245
out = db.daily_readiness_history(tmpdb, days=3, cfg=cfg)
246246
assert [r["date"] for r in out] == ["2026-05-03", "2026-05-04", "2026-05-05"]
247247

248248

249-
def test_recent_activities_cutoff_uses_local_today(tmpdb):
249+
def test_recent_activities_cutoff_useslocal_today(tmpdb):
250250
"""An activity dated as Pi-local 2026-05-05 must be findable when
251251
queried at local 2026-05-05, even when UTC has rolled over to 05-06."""
252252
from unittest.mock import patch
@@ -255,7 +255,7 @@ def test_recent_activities_cutoff_uses_local_today(tmpdb):
255255
"name": "easy run", "duration_s": 1800, "distance_m": 4000,
256256
"avg_hr": 130, "max_hr": 150, "calories": 300, "training_effect": 2.5,
257257
})
258-
with patch("src.db._local_today", return_value=date(2026, 5, 5)):
258+
with patch("src.db.local_today", return_value=date(2026, 5, 5)):
259259
out = db.recent_activities(tmpdb, days=1)
260260
assert any(a["activity_id"] == "a1" for a in out)
261261

tests/test_training_intel.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ def test_daily_readiness_history_returns_one_per_day(tmpdb):
268268
cfg = _make_cfg(tmpdb)
269269
out = db.daily_readiness_history(tmpdb, days=7, cfg=cfg)
270270
assert len(out) == 7
271-
# Production now anchors on Pi-local 'today' (db._local_today), so this
271+
# Production now anchors on Pi-local 'today' (db.local_today), so this
272272
# plain `date.today()` matches and is no longer UTC-flaky.
273273
today = date.today().isoformat()
274274
assert out[-1]["date"] == today

0 commit comments

Comments
 (0)