Skip to content

fix(proxy): remove ghost team BYOK models from /models on delete - #29529

Closed
aanchal22 wants to merge 1 commit into
BerriAI:litellm_oss_branchfrom
aanchal22:litellm_byok-delete-ghost-models
Closed

fix(proxy): remove ghost team BYOK models from /models on delete#29529
aanchal22 wants to merge 1 commit into
BerriAI:litellm_oss_branchfrom
aanchal22:litellm_byok-delete-ghost-models

Conversation

@aanchal22

@aanchal22 aanchal22 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Relevant issues

Addresses issue - #22594

Related PRs: #29528 (companion create-side fix)

Pre-Submission checklist

  • I have added meaningful tests
  • 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
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

Reproduction script creates a team with 2 BYOK models sequentially, then deletes them. Before the fix, both models persist as ghosts in team.models and /models indefinitely. After the fix, deleted models are removed immediately.

Before (v1.85.0):
  After deleting both models:
  team.models in DB: ['all-proxy-models', 'bug2-model-1', 'bug2-model-2']
  Ghosts: both models still visible via /models

After (this PR):
  After deleting model-2:
  team.models in DB: ['all-proxy-models', 'bug2-model-1']
  After deleting model-1:
  team.models in DB: ['all-proxy-models']
  No ghosts.

Also tested the alias-overwrite scenario: create model-1, create model-2 (overwrites model-1's alias entry), delete model-1. Before the fix, model-1 persists as a ghost. After the fix, model-1 is cleaned up and model-2 is preserved.

Type

Bug Fix

Changes

delete_model passed model_params.model_name (the internal name, e.g. model_name_{team_id}_{uuid}) to delete_team_model_alias, which searched alias map values for a match. For BYOK models the alias map is typically empty or was overwritten by a later model create, so the lookup always failed and the public name was never removed from team.models.

This PR adds two helpers and rewires the delete path:

  • _resolve_team_public_model_name: extracts the user-visible name from model_info.team_public_model_name, falling back to model_params.model_name for non-BYOK models.
  • _cleanup_team_model_references: builds a names_to_remove set starting with the public name (unconditionally), then adds any alias keys whose values match the internal name. Strips all matched names from team.models and cleans up the alias map in the same pass.

After the DB cleanup, refreshes the team cache via _cache_team_object so /models reflects the deletion without waiting for cache TTL. The cache refresh is wrapped in try/except so a Redis failure does not undo the DB delete.

delete_team_model_alias is left in place; other code paths may still reference it.

@aanchal22
aanchal22 requested a review from a team June 2, 2026 20:23
@aanchal22
aanchal22 force-pushed the litellm_byok-delete-ghost-models branch from 79f8dad to 726fb9c Compare June 2, 2026 20:31
@greptile-apps

greptile-apps Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes ghost team BYOK models persisting in team.models and /models after deletion by introducing two new helpers that correctly resolve the public model name and remove both the public name and any alias map entries pointing to the deleted internal model name.

  • _resolve_team_public_model_name extracts model_info.team_public_model_name (the user-visible name) instead of the internal model_name_{team_id}_{uuid} value, fixing the root cause of the ghost-model bug.
  • _cleanup_team_model_references atomically updates the alias map and team.models in a single pass, then refreshes the team cache via _cache_team_object with include={\"object_permission\": True, \"litellm_model_table\": True} so /models reflects the deletion immediately without waiting for TTL expiry.

Confidence Score: 5/5

Safe to merge — the change is well-scoped to the delete path, all three concerns raised in the prior review round have been addressed, and the new helpers are covered by focused mock tests.

All three previously-flagged issues are resolved: the team-table update now carries include object_permission and litellm_model_table so the cached object retains relational fields; the function returns None (not the initial find_unique row) when nothing was removed; and the test mock for litellm_teamtable.update now returns a proper row. The logic for building names_to_remove, cleaning the alias map, and refreshing the cache is correct across all code paths.

No files require special attention.

Important Files Changed

Filename Overview
litellm/proxy/management_endpoints/model_management_endpoints.py Adds _resolve_team_public_model_name and _cleanup_team_model_references helpers; rewires delete_model to use them and refresh the team cache with full relational includes after a DB change.
tests/test_litellm/proxy/management_endpoints/test_delete_team_byok_cleanup.py New mock-only unit tests covering empty-alias, populated-alias, no-change, and team-not-found cases for the two new helpers.

Reviews (3): Last reviewed commit: "fix(proxy): remove ghost team BYOK model..." | Re-trigger Greptile

Comment thread litellm/proxy/management_endpoints/model_management_endpoints.py
Comment thread litellm/proxy/management_endpoints/model_management_endpoints.py
@codecov

codecov Bot commented Jun 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.35897% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...management_endpoints/model_management_endpoints.py 74.35% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

@aanchal22
aanchal22 force-pushed the litellm_byok-delete-ghost-models branch from 726fb9c to 82fa10f Compare June 2, 2026 20:41
@Sameerlite

Copy link
Copy Markdown
Contributor

Please get the score to 5. You can tag the bot to get tit reviewed again

@aanchal22

Copy link
Copy Markdown
Contributor Author

@greptile-apps re-review please

delete_model passed the internal model_name (model_name_{team_id}_{uuid})
to delete_team_model_alias, which searched alias map values for it.
For BYOK models the alias map is typically empty, so the lookup always
failed and the public name was never removed from team.models; creating
a ghost model visible via /models indefinitely.

Adds _resolve_team_public_model_name to extract the user-visible name
from model_info.team_public_model_name. Adds _cleanup_team_model_references
which always strips the public name from team.models (regardless of alias
map state) and also cleans up any alias entries pointing to the deleted
model's internal name.

On behalf of Adobe Inc.
@aanchal22
aanchal22 force-pushed the litellm_byok-delete-ghost-models branch from 82fa10f to 3587d46 Compare June 3, 2026 14:33
@aanchal22

aanchal22 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@greptile-apps re-review

@aanchal22

Copy link
Copy Markdown
Contributor Author

@greptile-apps re-review please

@krrish-berri-2

Copy link
Copy Markdown
Contributor

@aanchal22 — could you add a screenshot or short video showing that this change works as expected (e.g. before/after: ghost team BYOK models no longer appearing in /models after delete)? It really helps reviewers verify the fix quickly. Thanks!

@aanchal22

aanchal22 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Setup -

BASE=http://localhost:4000

Before fix (local stack stems from BerriAI:litellm_oss_branch):

Create 2 models -
Screenshot 2026-06-03 at 4 37 26 PM
Models still listed after successful delete -
Screenshot 2026-06-03 at 4 37 38 PM

After fix (local stack stems from aanchal22:litellm_byok-delete-ghost-models):

Deletes both models successfully -
Screenshot 2026-06-03 at 4 41 04 PM

@aanchal22

aanchal22 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Closing this as solved by #29820

@aanchal22 aanchal22 closed this Jun 11, 2026
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.

3 participants