code review: update track_cost handling in ModelSettings and add related tests

Author: habema

src/agents/model_settings.py

@@ -120,11 +120,11 @@ class ModelSettings:
     """Whether to include usage chunk.
     Only available for Chat Completions API."""
 
-    track_cost: bool = False
+    track_cost: bool | None = None
     """Whether to track and calculate cost for model calls.
     When enabled, the SDK will populate `Usage.cost` with cost estimates.
     Currently only supported for LiteLLM models. For other providers, cost will remain None.
-    Defaults to False."""
+    If not provided (i.e., set to None), cost tracking is disabled (defaults to False)."""
 
     # TODO: revisit ResponseIncludable | str if ResponseIncludable covers more cases
     # We've added str to support missing ones like

tests/model_settings/test_serialization.py

@@ -57,6 +57,7 @@ def test_all_fields_serialization() -> None:
         metadata={"foo": "bar"},
         store=False,
         include_usage=False,
+        track_cost=False,
         response_include=["reasoning.encrypted_content"],
         top_logprobs=1,
         verbosity="low",

tests/models/test_litellm_cost_tracking.py

@@ -49,7 +49,7 @@ def fake_completion_cost(completion_response):
 @pytest.mark.allow_call_model_methods
 @pytest.mark.asyncio
 async def test_cost_none_when_track_cost_disabled(monkeypatch):
-    """Test that cost is None when track_cost=False (default)."""
+    """Test that cost is None when track_cost is not set (defaults to None/False)."""
 
     async def fake_acompletion(model, messages=None, **kwargs):
         msg = Message(role="assistant", content="Test response")
@@ -61,13 +61,13 @@ async def fake_acompletion(model, messages=None, **kwargs):
         return response
 
     monkeypatch.setattr(litellm, "acompletion", fake_acompletion)
-    # Note: completion_cost should not be called when track_cost=False
+    # Note: completion_cost should not be called when track_cost is None (default)
 
     model = LitellmModel(model="test-model", api_key="test-key")
     result = await model.get_response(
         system_instructions=None,
         input=[],
-        model_settings=ModelSettings(track_cost=False),  # Disabled (default)
+        model_settings=ModelSettings(),  # track_cost defaults to None (disabled)
         tools=[],
         output_schema=None,
         handoffs=[],
@@ -192,3 +192,29 @@ def fake_completion_cost(completion_response):
     assert result.usage.total_tokens == 150
     assert result.usage.cost == 0.001
     assert result.usage.requests == 1
+
+
+def test_track_cost_sticky_through_resolve():
+    """Test that track_cost=True is not overwritten by resolve() with empty override."""
+    base = ModelSettings(track_cost=True, temperature=0.7)
+    override = ModelSettings(max_tokens=100)  # Only setting max_tokens, track_cost is None
+
+    resolved = base.resolve(override)
+
+    # track_cost should remain True because override's track_cost is None (not False)
+    assert resolved.track_cost is True
+    assert resolved.temperature == 0.7
+    assert resolved.max_tokens == 100
+
+
+def test_track_cost_can_be_explicitly_disabled():
+    """Test that track_cost=True can be explicitly overridden to False."""
+    base = ModelSettings(track_cost=True, temperature=0.7)
+    override = ModelSettings(track_cost=False, max_tokens=100)
+
+    resolved = base.resolve(override)
+
+    # track_cost should be False because override explicitly set it to False
+    assert resolved.track_cost is False
+    assert resolved.temperature == 0.7
+    assert resolved.max_tokens == 100