feat: default cost estimates and rate updater #13
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements default cost estimation and a rate updater system to enhance cost tracking capabilities. The changes enable automatic cost calculation when usage tracking is enabled, with bundled rate data that can be refreshed through a scheduled GitHub Action.
Key changes:
- Automatic cost estimation by default when
track_usage=Truewith opt-out viaauto_cost=False - New
rate_last_updatedfield inUsageMetricsto expose pricing data freshness - Bundled rate data in JSON format with fallback defaults and a script to update rates weekly
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/cellsem_llm_client/tracking/usage_metrics.py |
Added rate_last_updated field to track when pricing data was last updated |
tests/unit/test_usage_metrics.py |
Updated tests to verify the new rate_last_updated field |
src/cellsem_llm_client/tracking/rates.json |
New bundled JSON file containing current pricing data for OpenAI and Anthropic models |
src/cellsem_llm_client/tracking/cost_calculator.py |
Modified to load rates from JSON file with fallback to embedded defaults |
src/cellsem_llm_client/agents/agent_connection.py |
Added auto-cost calculator creation, rate freshness tracking, and auto_cost parameter |
scripts/update_rates.py |
New script to update bundled rate data with current UTC timestamps |
pyproject.toml |
Added package data configuration to include rates.json in distribution |
planning/cost_tracking_improvements.md |
Planning document describing the scope and tasks for cost tracking enhancements |
docs/index.md |
Added link to new cost tracking documentation |
docs/cost_tracking.md |
Comprehensive guide for estimated and actual cost tracking |
README.md |
Updated to reflect automatic cost estimation and link to documentation |
.github/workflows/update_rates.yml |
Weekly GitHub Action to automatically update rate data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SchemaManager, | ||
| SchemaValidator, | ||
| ) | ||
| from cellsem_llm_client.tracking.cost_calculator import FallbackCostCalculator |
There was a problem hiding this comment.
The FallbackCostCalculator is imported both here and in the TYPE_CHECKING block (line 36). Since it's used at runtime (line 580), the import on line 21 is correct, but the TYPE_CHECKING import on line 36 is now redundant and can be removed.
| track_usage: bool = False, | ||
| cost_calculator: Optional["FallbackCostCalculator"] = None, | ||
| max_retries: int = 2, | ||
| auto_cost: bool = True, |
There was a problem hiding this comment.
The new auto_cost parameter lacks test coverage. Consider adding tests that verify: (1) when auto_cost=True and cost_calculator=None, a default calculator is created and cost estimation is performed; (2) when auto_cost=False, no automatic calculator is created; (3) when both cost_calculator is provided and auto_cost=True, the provided calculator is used.
| rate_last_updated = None | ||
| if cost_calculator: | ||
| try: | ||
| get_rates = getattr(cost_calculator, "get_model_rates", None) | ||
| rate_data = ( | ||
| get_rates(provider, self.model) if callable(get_rates) else None | ||
| ) | ||
| if rate_data and hasattr(rate_data, "source"): | ||
| access_date = getattr(rate_data.source, "access_date", None) | ||
| rate_last_updated = ( | ||
| access_date if isinstance(access_date, datetime) else None | ||
| ) |
There was a problem hiding this comment.
The new rate_last_updated field extraction logic (lines 686-697) lacks test coverage. Consider adding a test that verifies the rate_last_updated field is correctly populated in the usage metrics when a cost calculator with rate data is provided.
| get_rates = getattr(cost_calculator, "get_model_rates", None) | ||
| rate_data = ( | ||
| get_rates(provider, self.model) if callable(get_rates) else None | ||
| ) | ||
| if rate_data and hasattr(rate_data, "source"): | ||
| access_date = getattr(rate_data.source, "access_date", None) | ||
| rate_last_updated = ( | ||
| access_date if isinstance(access_date, datetime) else None | ||
| ) |
There was a problem hiding this comment.
The logic for extracting rate_last_updated from the cost calculator (lines 689-697) is duplicated in _accumulate_usage_metrics (lines 780-788). Consider extracting this into a helper method to reduce duplication and improve maintainability.
| fallback_rates = [ | ||
| ModelCostData( | ||
| provider="openai", | ||
| model="gpt-4", | ||
| input_cost_per_1k_tokens=0.03, | ||
| output_cost_per_1k_tokens=0.06, | ||
| source=default_source, | ||
| ), | ||
| ModelCostData( | ||
| provider="openai", | ||
| model="gpt-3.5-turbo", | ||
| input_cost_per_1k_tokens=0.0015, | ||
| output_cost_per_1k_tokens=0.002, | ||
| source=default_source, | ||
| ), | ||
| ModelCostData( | ||
| provider="openai", | ||
| model="gpt-4o-mini", | ||
| input_cost_per_1k_tokens=0.00015, | ||
| output_cost_per_1k_tokens=0.0006, | ||
| cached_cost_per_1k_tokens=0.000075, | ||
| source=default_source, | ||
| ), | ||
| ModelCostData( | ||
| provider="anthropic", | ||
| model="claude-3-sonnet", | ||
| input_cost_per_1k_tokens=0.003, | ||
| output_cost_per_1k_tokens=0.015, | ||
| thinking_cost_per_1k_tokens=0.006, | ||
| source=default_source, | ||
| ), | ||
| ModelCostData( | ||
| provider="anthropic", | ||
| model="claude-3-haiku-20240307", | ||
| input_cost_per_1k_tokens=0.00025, | ||
| output_cost_per_1k_tokens=0.00125, | ||
| thinking_cost_per_1k_tokens=0.0005, | ||
| source=default_source, | ||
| ), | ||
| ] |
There was a problem hiding this comment.
The fallback_rates list is missing the "gpt-4o" model that is present in the rates.json file (lines 17-27). If the JSON file fails to load, users won't be able to get cost estimates for gpt-4o. Consider adding this model to the fallback list for consistency.
| rate = ModelCostData(source=source, **entry) | ||
| self._rate_database[(rate.provider, rate.model)] = rate | ||
| return | ||
| except Exception: |
There was a problem hiding this comment.
Using a bare except Exception silently swallows all errors, making debugging difficult. Consider catching specific exceptions (e.g., FileNotFoundError, json.JSONDecodeError, KeyError) or at least logging the exception before falling back to defaults.
No description provided.