Skip to content

refactor: Remove redundant backup file - #16590

Merged
Chesars merged 14 commits into
BerriAI:litellm_staging_03_21_2026from
Chesars:refactor/remove-backup-file-dry-principle
Mar 22, 2026
Merged

refactor: Remove redundant backup file#16590
Chesars merged 14 commits into
BerriAI:litellm_staging_03_21_2026from
Chesars:refactor/remove-backup-file-dry-principle

Conversation

@Chesars

@Chesars Chesars commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Title

Remove redundant backup file for model pricing

Relevant issues

Closes #16460

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/litellm/ directory, Adding at least 1 test is a hard
    requirement
    - see details
  • I have added a screenshot of my new test passing locally
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem

Type

🧹 Refactoring

Changes

Summary

Removed the redundant litellm/model_prices_and_context_window_backup.json file and refactored the codebase to use a single source of truth for model pricing data.

Solution

  • Removed litellm/model_prices_and_context_window_backup.json (24,718 lines deleted)
  • Refactored get_model_cost_map() to read from single source:
    • Development: Reads from project root model_prices_and_context_window.json
    • Production: Reads from package resources (copied by CI during publish)
  • Updated CI/CD:
    • .circleci/config.yml: Changed to copy main file to package before publish
    • .github/workflows/simple_pypi_publish.yml: Same approach
    • ci_cd/check_files_match.py: Updated file paths
  • Updated tests: Renamed and refactored test_get_local_model_cost_map()

How it works

  1. Development: Code reads model_prices_and_context_window.json from project root
  2. CI/CD: Before publishing to PyPI, copies file to litellm/ directory
  3. Production: Users get file bundled in package after pip install litellm

Benefits

Testing

  • ✅ Local mode works: LITELLM_LOCAL_MODEL_COST_MAP=True
  • ✅ Fallback works when GitHub download fails
  • ✅ All existing tests pass
  • ✅ Loads 1,743 models successfully

✓ Loaded successfully
Models count: 1743

Tests passing:
tests/local_testing/test_get_model_file.py::test_get_local_model_cost_map PASSED
tests/local_testing/test_get_model_file.py::test_get_model_cost_map PASSED
tests/test_litellm/test_cost_calculator.py::test_cost_calculator_with_usage PASSED

- Removed litellm/model_prices_and_context_window_backup.json
- Updated get_model_cost_map() to read from single source
- Refactored _load_local_model_cost_map() to support:
  * Package resources (production/pip install)
  * Project root (development)
- Updated CI/CD workflows:
  * .circleci/config.yml: Copy to litellm/ before publishing
  * .github/workflows/simple_pypi_publish.yml: Same approach
  * ci_cd/check_files_match.py: Updated file paths
- Updated test_get_model_file.py to use project root
- Renamed test_get_backup_model_cost_map → test_get_local_model_cost_map

Benefits:
- Eliminates file duplication
- Single source of truth for model pricing
- No need for PRs like BerriAI#16460 to sync backup files
- Simpler maintenance

The backup was unnecessary because CI copies the file to the package before publishing.
@vercel

vercel Bot commented Nov 13, 2025

Copy link
Copy Markdown

@Chesars is attempting to deploy a commit to the CLERKIEAI Team on Vercel.

A member of the Team first needs to authorize it.

@Chesars

Chesars commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

@greptile review commit 88ee7c5

@greptile-apps

greptile-apps Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR eliminates the long-standing dual-file maintenance burden by removing litellm/model_prices_and_context_window_backup.json and refactoring get_model_cost_map.py to load from a single canonical file — first attempting package resources (production), then falling back to the project root (development). CI/CD pipelines are updated to copy the root-level JSON into the litellm/ package directory only at publish time, and the now-redundant check_files_match.py pre-commit hook is removed.

  • Core loading logic (get_model_cost_map.py): load_local_model_cost_map() now uses importlib.resources.files().joinpath().read_text() (the modern, non-deprecated API) with a project-root fallback via pathlib.Path. All internal naming updated from _backup_*_local_*.
  • CI/CD (.circleci/config.yml, simple_pypi_publish.yml): cp destination updated from litellm/model_prices_and_context_window_backup.jsonlitellm/model_prices_and_context_window.json.
  • .gitignore: litellm/model_prices_and_context_window.json correctly added on its own line (missing trailing newline on the previous last line was also fixed).
  • Pre-commit hook removed: ci_cd/check_files_match.py deleted and its hook entry removed from .pre-commit-config.yaml.
  • Tests updated: test_get_backup_model_cost_map replaced with test_get_local_model_cost_map which now exercises the actual production loading path. _load_backup_json() in test_deepseek_model_metadata.py renamed to _load_model_cost_json() and delegates to GetModelCostMap.load_local_model_cost_map().
  • Minor: Three imports (traceback, importlib.resources, json) are now unused in tests/local_testing/test_get_model_file.py after the test was rewritten and should be removed.

Confidence Score: 4/5

  • This PR is safe to merge — the refactor is well-scoped, CI/CD and .gitignore are correctly updated, and the only remaining open items are narrow exception handling (already flagged in prior review rounds) and a trivial unused-import cleanup.
  • The single-source-of-truth goal is cleanly achieved. The loading path logic is correct for both development and production environments. All previously raised concerns about .gitignore (broken newline), the pre-commit hook breaking developer commits, and the stale _load_backup_json name have been addressed in this PR. The remaining exception-handling narrowness (FileNotFoundError, ModuleNotFoundError only) was flagged in prior threads and does not block merging. The only new finding is three unused imports — a trivially fixable style issue.
  • litellm/litellm_core_utils/get_model_cost_map.py — exception handling in load_local_model_cost_map() remains narrow (pre-existing open issue); tests/local_testing/test_get_model_file.py — three unused imports to clean up.

Important Files Changed

Filename Overview
litellm/litellm_core_utils/get_model_cost_map.py Core loading logic refactored to try package resources first, then fall back to project root. Exception handling remains narrow (FileNotFoundError, ModuleNotFoundError only) — remaining open concerns from previous threads.
tests/local_testing/test_get_model_file.py Test now correctly exercises the production loading path via GetModelCostMap.load_local_model_cost_map(). Three imports (traceback, importlib.resources, json) are now unused and should be removed.
tests/test_litellm/test_deepseek_model_metadata.py _load_backup_json() renamed to _load_model_cost_json() and now delegates to GetModelCostMap.load_local_model_cost_map(). Tests are equivalent in coverage with a cleaner loading path.
.gitignore Properly adds litellm/model_prices_and_context_window.json on a new line (the missing trailing newline from the original file was fixed). Both patterns are now valid.
.pre-commit-config.yaml Correctly removes the now-unnecessary check-files-match pre-commit hook alongside the deletion of ci_cd/check_files_match.py.
ci_cd/check_files_match.py File deleted as expected — the sync check is no longer needed with a single source of truth.
.circleci/config.yml CI step updated to copy model_prices_and_context_window.json → litellm/model_prices_and_context_window.json before publish instead of the old backup filename.
.github/workflows/simple_pypi_publish.yml GitHub Actions publish workflow updated in lockstep with CircleCI — copies to the new canonical package path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[get_model_cost_map called] --> B{LITELLM_LOCAL_MODEL_COST_MAP=True?}
    B -- Yes --> C[load_local_model_cost_map]
    B -- No --> D[fetch_remote_model_cost_map URL]
    D -- Network error --> C
    D -- Success --> E{validate_model_cost_map}
    E -- Fail --> C
    E -- Pass --> F[Return remote map + expand aliases]
    C --> G{Try: importlib.resources\nfiles litellm .joinpath\nmodel_prices_and_context_window.json}
    G -- FileNotFoundError /\nModuleNotFoundError --> H[Fallback: Path __file__ .parent.parent.parent\nmodel_prices_and_context_window.json]
    G -- Success --> I[Return package resource map]
    H --> J[Return project-root map]
    I --> K[expand_model_aliases + return]
    J --> K
Loading

Comments Outside Diff (1)

  1. tests/local_testing/test_get_model_file.py, line 1-3 (link)

    Stale imports left behind after refactor

    After removing test_get_backup_model_cost_map, three top-level imports are now unused: traceback, importlib.resources, and json. The new test_get_local_model_cost_map only uses a locally-scoped from litellm... import. Linters (e.g. flake8 F401) will warn on these.

Last reviewed commit: "Merge remote-trackin..."

Comment thread litellm/litellm_core_utils/get_model_cost_map.py Outdated
Comment thread litellm/litellm_core_utils/get_model_cost_map.py Outdated
Comment thread tests/local_testing/test_get_model_file.py Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@vercel

vercel Bot commented Mar 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Mar 22, 2026 2:58am

Request Review

Comment thread ci_cd/check_files_match.py Outdated
The hook kept the backup JSON in sync with the root file — no longer
needed now that the backup file has been removed.
Comment thread litellm/litellm_core_utils/get_model_cost_map.py Outdated
Comment thread litellm/litellm_core_utils/get_model_cost_map.py Outdated
Comment thread .circleci/config.yml
Chesars added 2 commits March 20, 2026 22:48
- Update load_local_model_cost_map to use project root fallback for dev
- Keep main's validation, aliases, and source info tracking
- Remove backup JSON (purpose of this PR)
Prevent accidental commits of the CI-generated copy.
Comment thread .gitignore Outdated
…or/remove-backup-file-dry-principle

# Conflicts:
#	litellm/model_prices_and_context_window_backup.json
@codspeed-hq

codspeed-hq Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing Chesars:refactor/remove-backup-file-dry-principle (556a1ae) with main (f5194b5)

Open in CodSpeed

Comment thread tests/test_litellm/test_deepseek_model_metadata.py Outdated
Chesars and others added 2 commits March 20, 2026 23:14
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…e-backup-file-dry-principle

# Conflicts:
#	litellm/model_prices_and_context_window_backup.json
@Chesars
Chesars changed the base branch from main to litellm_staging_03_21_2026 March 22, 2026 02:57
@Chesars
Chesars merged commit 62df632 into BerriAI:litellm_staging_03_21_2026 Mar 22, 2026
35 of 39 checks passed
@Chesars
Chesars deleted the refactor/remove-backup-file-dry-principle branch March 22, 2026 02:58
Chesars added a commit that referenced this pull request Apr 25, 2026
…-file-dry-principle"

This reverts commit 62df632, reversing
changes made to a646214.
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…file-dry-principle

refactor: Remove redundant backup file
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…-backup-file-dry-principle"

This reverts commit 62df632, reversing
changes made to a646214.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants