-
Couldn't load subscription status.
- Fork 14
User-defined cost tracker contexts #355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces user-defined cost tracking contexts, enabling users to scope expensive operations to custom CostTracker instances instead of the global tracker. Custom trackers are enabled by default, while the global tracker is now disabled by default. Users can instantiate a CostTracker and use it as a context manager to isolate cost tracking for specific operations.
Key Changes:
- Added context manager protocol (
__enter__/__exit__) toCostTrackerfor scoped cost tracking - Introduced module-level
_active_trackercontext variable to manage active tracker state - Changed default behavior: custom trackers enabled by default, global tracker disabled by default
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/lmi/src/lmi/cost_tracker.py | Core implementation of custom tracker contexts, context variable management, and updated default behavior |
| packages/lmi/tests/test_cost_tracking.py | Comprehensive test suite covering custom tracker defaults, basic usage, LLM integration, nested/sequential trackers, and parallel async task isolation |
| packages/lmi/tests/cassettes/*.yaml | VCR cassettes for test_custom_tracker_with_llm_calls with OpenAI and Anthropic models in streaming and non-streaming modes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
| """Enable or disable cost tracking for this tracker.""" | ||
| self.enabled.set(enabled) | ||
|
|
||
| def __enter__(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def __enter__(self): | |
| def __enter__(self) -> Self: |
|
|
||
| def _get_active_tracker() -> "CostTracker": | ||
| """Get the currently active cost tracker, defaulting to GLOBAL_COST_TRACKER.""" | ||
| return _active_tracker.get() or GLOBAL_COST_TRACKER |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on this and the PR description, this is where we have to choose between a temporary cost tracker and the global cost tracker?
I can see arguments for both sides (switching between global cost tracking vs always adding to it).
For me, I think having global cost tracking always on is the most easy/predictable. To do a sum across different cost trackers could get arduous.
What did you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah after talking about it, I'm on the same page. Will do.
| # Global tracker should remain unchanged | ||
| assert GLOBAL_COST_TRACKER.lifetime_cost_usd == global_initial_cost | ||
|
|
||
| @pytest.mark.vcr(match_on=VCR_DEFAULT_MATCH_ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @pytest.mark.vcr(match_on=VCR_DEFAULT_MATCH_ON) | |
| @pytest.mark.vcr |
I think this is basically re-specifying the defaults, unless I am missing something
Allows a user to scope certain operations in a cost-tracking context, instead of adding it to the global total:
Up for discussion: if inside a custom tracker context, should we still add to
GLOBAL_COST_TRACKER?