feat: new refresh_prices method for synchronous data fetching
#109
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new synchronous refresh_prices method for updating token prices and removes deprecated llama-index-related code. Key changes include:
- Adding a new refresh_prices method in tokencost/constants.py that wraps the asynchronous update_token_costs call.
- Updating update_prices.py to use the new refresh_prices method and improving output messages.
- Removing llama-index callback support along with associated tests and dependency references.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| update_prices.py | Uses refresh_prices and refactors file update logic. |
| tokencost/constants.py | Adds refresh_prices synchronous wrapper. |
| tokencost/callbacks/llama_index.py | Removes deprecated llama-index callback handler. |
| tokencost/init.py | Updates module exports to include refresh_prices. |
| tests/test_llama_index_callbacks.py | Removes tests for removed llama-index code. |
| tach.yml | Removes dependency on llama-index callbacks. |
| pyproject.toml | Updates version, Python requirement, and team members. |
Files not reviewed (1)
- tox.ini: Language not supported
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new synchronous method, refresh_prices, for updating token costs and refactors the update_prices workflow to utilize it while removing deprecated llama-index code.
- Added refresh_prices as a synchronous wrapper for update_token_costs that optionally writes to model_prices.json
- Updated update_prices.py to call refresh_prices and adjusted file update logic and formatting
- Removed all llama-index related code and tests; version and dependency updates in pyproject.toml
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| update_prices.py | Uses refresh_prices for fetching token prices and updates the file |
| tokencost/constants.py | Adds refresh_prices; provides synchronous wrapper for token update |
| tokencost/callbacks/llama_index.py | Removed deprecated llama-index callback handler |
| tokencost/init.py | Updated import to include refresh_prices |
| tests/test_llama_index_callbacks.py | Removed test file for llama-index callbacks |
| tach.yml | Removed dependency on llama-index callbacks |
| pyproject.toml | Bumped version, updated authors, and removed llama-index dependency |
Files not reviewed (1)
- tox.ini: Language not supported
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new synchronous method, refresh_prices, to fetch and update token costs and updates update_prices.py to use it while removing deprecated llama-index code.
- Added refresh_prices in tokencost/constants.py to synchronously wrap update_token_costs.
- Updated update_prices.py to leverage refresh_prices and improved log messages.
- Removed deprecated llama-index callbacks and tests, and updated project configuration (version and Python requirement).
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| update_prices.py | Uses refresh_prices and improves diff_dicts and logging behavior |
| tokencost/constants.py | Introduces refresh_prices synchronous method and updates error handling |
| tokencost/callbacks/llama_index.py | Removes obsolete llama-index callback code |
| tokencost/init.py | Exposes refresh_prices in the public API |
| tests/test_llama_index_callbacks.py | Removes tests for the deprecated llama-index callbacks |
| tach.yml | Removes dependency on the llama-index callbacks module |
| pyproject.toml | Bumps version to 0.2.0 and updates the Python requirement |
Files not reviewed (1)
- tox.ini: Language not supported
dot-agi
force-pushed
the
feat/new-prices-fetch
branch
from
379aa2a to
17b3c5e
Compare
This was
linked to
issues
May 7, 2025
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog
Closes #107 #106 #93 #81 #74
refresh_pricesmethod that usesupdate_token_costsand optionally writes tomodel_prices.jsonupdate_prices.pyscript to use this methodllama-index