Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.30%. Comparing base (c82d28b) to head (cdc6461).
⚠️ Report is 3 commits behind head on main.

@@            Coverage Diff             @@
##             main      #60      +/-   ##
==========================================
+ Coverage   83.27%   83.30%   +0.03%     
==========================================
  Files          10       10              
  Lines        1363     1366       +3     
==========================================
+ Hits         1135     1138       +3     
  Misses        228      228              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hey @jasisz — thanks for this! Substantive change looks great; verified all 485 tests pass locally on the branch, and the caching semantics match our access pattern cleanly (the system prompt is f"{SYSTEM_PROMPT}\n\n{skill_md}" — byte-identical across all 60 problems in a run).

Quick heads-up: CodeRabbit flagged one outside-diff finding you may have missed — it's stashed in the review's "⚠️ Outside diff range comments" collapsible on this review rather than posted inline, which makes it easy to skip.

The finding: with cache_control: ephemeral enabled, Anthropic splits response.usage into three input-token counters:

The current LLMResponse only captures input_tokens, which after this PR merges will drop to ~1-2k per call (user message only). Downstream consumers (runner.py → ProblemResult.input_tokens → JSONL logs) would then show misleadingly tiny numbers — anyone doing cost analysis from the JSONLs would be off by ~10×.

Since the whole point of this PR is cost reduction, getting the accounting right here feels worth including. Would you mind folding in CodeRabbit's suggested Option B on the same PR?

elapsed = time.monotonic() - start
text = response.content[0].text if response.content else ""
usage = response.usage
cache_creation = getattr(usage, "cache_creation_input_tokens", 0) or 0
cache_read = getattr(usage, "cache_read_input_tokens", 0) or 0
return LLMResponse(
    text=text,
    input_tokens=usage.input_tokens + cache_creation + cache_read,
    output_tokens=response.usage.output_tokens,
    wall_time_s=round(elapsed, 2),
    model=response.model,
)

One gotcha for the test: tests/test_models.py::TestAnthropicClient::test_complete_mock explicitly sets mock_resp.usage.input_tokens = 100 but doesn't set the cache fields. MagicMock auto-vivifies unset attributes as further MagicMocks (truthy, not int), so usage.input_tokens + cache_creation + cache_read would blow up on addition. Easy fix — just add:

mock_resp.usage.cache_creation_input_tokens = 0
mock_resp.usage.cache_read_input_tokens = 0

to the mock setup around line 105.

Happy to push that as a commit on your branch if you've got "Allow edits from maintainers" enabled and that's easier — or you can roll it in yourself, whichever you prefer. Option A (separate cache_creation_input_tokens / cache_read_input_tokens fields on LLMResponse) would be nicer for proper cost analysis later, but it's a bigger surface-area change — I'd treat that as a follow-up if we want it.

Good catch — pushed the accounting fix in cdc6461. Didn't realize Anthropic splits the usage counters when caching is enabled, that's a gotcha worth knowing about. Thanks for the detailed breakdown!

Follow-up for the other providers filed as #61 — OpenAI has automatic caching already firing on our calls (we're just not instrumenting it, so the savings aren't visible in the JSONLs), and Moonshot has Context Caching but needs a bigger refactor to wire in. Phase 1 there (OpenAI cached-token visibility) is a small parallel of what you did here; Phase 2 (Moonshot) is parked until benchmark cadence justifies it.