Repository navigation
Litellm ishaan april1 try2 - #25110
2 commits merged into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
ishaan-berri seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
* fix(proxy): enforce upperbound key params on key/update and add custom_key_update hook The /key/update endpoint did not enforce upperbound_key_generate_params, allowing users to bypass configured limits (tpm_limit, rpm_limit, max_budget, duration, budget_duration) by updating an existing key instead of generating a new one. Extract the upperbound enforcement logic from _common_key_generation_helper() into a standalone _enforce_upperbound_key_params() function and call it from both the generate and update paths. For updates, None values are skipped (not filled with defaults) since they mean "don't change this field". Also adds a custom_key_update config option and user_custom_key_update global, mirroring the existing custom_key_generate pattern, so custom key validation logic can fire during key updates as well. * fix(proxy): invoke custom_key_update hook in bulk update path The user_custom_key_update hook was only called in update_key_fn (single key update) but not in _process_single_key_update (bulk update path), allowing custom validation to be bypassed via the /key/update/bulk endpoint. Mirror the hook invocation in both paths. * fix(proxy): pass UpdateKeyRequest to hook in bulk path, not BulkUpdateKeyRequestItem Move the custom_key_update hook invocation to after UpdateKeyRequest is constructed so the hook receives the same type in both single and bulk update paths. Previously the bulk path passed BulkUpdateKeyRequestItem (5 fields only), which would cause AttributeError for hooks accessing fields like tpm_limit or models. * fix(bedrock): promote cache usage to message_delta for Claude Code (#24850) Ensure Bedrock/Anthropic-compatible streaming exposes cache usage where Claude Code reads it by promoting message_stop usage onto message_delta and preserving usage fields in fake-streamed message_delta events. Made-with: Cursor * fix(search): Support self-hosted Firecrawl response format in search transform (#24866) The `transform_search_response` method only handled Firecrawl Cloud (v2) response format where `data` is a dict with `web`/`news` keys. Self-hosted Firecrawl (v1) returns `data` as a flat list of result objects, causing an `AttributeError: 'list' object has no attribute 'get'`. Detect the response format by checking if `data` is a list (self-hosted) or dict (cloud) and handle both cases. Cloud format: {"data": {"web": [...], "news": [...]}} Self-hosted: {"success": true, "data": [{"url": "...", "title": "...", ...}]} Co-authored-by: Synergy <synergyoclaw@gmail.com> * feat: add environment and user tracking to prompt management (#24855) * feat: add environment and user tracking to prompt management - Add environment (development/staging/production) and created_by columns to LiteLLM_PromptTable - Update unique constraint to [prompt_id, version, environment] - All CRUD endpoints support environment filtering and user tracking - Redesigned prompt detail page with environment tabs and version history - UI: environment filter on list page, environment selector in editor - 8 new tests for environment and user tracking Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: Black formatting and add environments to PromptInfoResponse TypeScript type Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address Greptile review findings - P1: delete_prompt scopes in-memory cleanup to environment when provided - P2: dotprompt_content parsed directly regardless of environment flag - P2: use distinct for environments query - P2: fix double-fetch on initial mount in prompt_info.tsx - fix: remove unsupported select kwarg from find_many Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address remaining Greptile review comments - Remove unused useCallback import (index.tsx) - Remove unused ENV_COLORS variable (prompt_info.tsx) - P1: in-memory fallback in get_prompt_versions now respects environment filter - P1: reset selectedEnv when promptId changes to avoid stale state - Cyclic imports are pre-existing pattern, not introduced by this PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: scope patch_prompt to environment using primary key - Add environment query param to patch_prompt endpoint - Look up target row by composite key (prompt_id + version + environment) - Update by primary key (id) to target exactly one row - Fixes Greptile finding: patch with multiple environments no longer ambiguous Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use actual start_time for failed request spend logs (#24906) async_post_call_failure_hook set both start_time and end_time to datetime.now(), making all failed requests show duration=0. Use the actual start_time from litellm_logging_obj instead, so spend logs reflect the real request duration on timeout and other failures. Fixes #24888 * feat(bedrock): add nova canvas image edit support (#24869) * feat(bedrock): add nova canvas image edit support * fix(bedrock): support PathLike inputs for nova image edit * chore: sync schema.prisma copies from root * fix(mypy): correct type-ignore code for delta_usage arg-type * fix(mypy): cast status_code to str, suppress intentional str yield * fix(lint): extract _create_content_block_chunks to fix PLR0915 * fix(lint): extract helpers to fix PLR0915 in prompt endpoints --------- Co-authored-by: michelligabriele <gabriele.michelli@icloud.com> Co-authored-by: Sameer Kankute <sameer@berri.ai> Co-authored-by: redhelix <amin.lalji@gmail.com> Co-authored-by: Synergy <synergyoclaw@gmail.com> Co-authored-by: Talha Anwar <37379131+talhaanwarch@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: madhu19991 <madhu@thunkai.com> Co-authored-by: Srikanth @adobe <devarakondasrikanth@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2f7ebea to
cc8ed6e
Compare
Greptile SummaryThis PR is a multi-feature release covering: (1) prompt environment/versioning support — new Key findings:
Confidence Score: 4/5Safe to merge after fixing the Edit button null-template bug; all server-side changes are solid but the UI has a functional issue that could silently open the editor with no content. Score of 4 reflects one P1 UI bug (Edit on a non-selected version passes null template, so the editor opens empty) and two P2 issues (double fetch on initial load, hardcoded environment filter). Server-side logic — migration, endpoint changes, key management refactor, Bedrock transforms, and Firecrawl support — is well-tested and correct. ui/litellm-dashboard/src/components/prompts/prompt_info.tsx (P1 Edit button bug and double-fetch); ui/litellm-dashboard/src/components/prompts.tsx (hardcoded env filter options)
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/prompts/prompt_info.tsx | Major refactor adding environment tabs, version history table, and per-version edit; has a P1 bug where clicking Edit on a non-selected version passes null for raw_prompt_template, and a P2 double-fetch issue with the isInitialMount pattern. |
| ui/litellm-dashboard/src/components/prompts.tsx | Adds environment filter Select with hardcoded options (development/staging/production); dynamic population from loaded prompts would be more robust. |
| litellm/proxy/prompts/prompt_endpoints.py | Adds environment and created_by scoping to all CRUD endpoints; get_prompt_versions switches from in-memory to DB queries which silently returns empty list when prisma_client is None. |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Refactors upperbound key param enforcement into _enforce_upperbound_key_params helper (correctly preserving behavior), and adds user_custom_key_update hook support for single and bulk key updates. |
| litellm-proxy-extras/litellm_proxy_extras/migrations/20260331000000_add_prompt_environment_and_created_by/migration.sql | Adds environment (default 'development') and created_by columns, replaces unique(prompt_id, version) with unique(prompt_id, version, environment); safe for existing data since all rows get the same default environment. |
| litellm/llms/bedrock/image_edit/amazon_nova_canvas_image_edit_transformation.py | New Nova Canvas image edit transformation supporting INPAINTING, OUTPAINTING, IMAGE_VARIATION, and BACKGROUND_REMOVAL task types; model support detected via model_cost JSON flag following existing patterns. |
| litellm/llms/bedrock/messages/invoke_transformations/anthropic_claude3_transformation.py | Adds _promote_message_stop_usage to merge Bedrock's cache usage fields from message_stop onto message_delta so SDK clients that only read message_delta see the full token breakdown. |
| litellm/proxy/proxy_server.py | Adds user_custom_key_update global variable, loaded from general_settings.custom_key_update config, following the same pattern as user_custom_key_generate. |
| litellm/proxy/hooks/proxy_track_cost_callback.py | Fixes failed request timing by using actual start_time from the logging object instead of datetime.now(), ensuring correct duration is recorded for failed requests. |
Reviews (1): Last reviewed commit: "fix(test): update model armor streaming ..." | Re-trigger Greptile
| e.stopPropagation(); | ||
| // Build a response-like object for the editor | ||
| const editData = { | ||
| prompt_spec: { | ||
| ...v, | ||
| prompt_id: basePromptId, | ||
| environment: selectedEnv, | ||
| }, | ||
| raw_prompt_template: isSelected ? promptTemplate : null, | ||
| }; | ||
| onEdit?.(editData); | ||
| }} | ||
| > | ||
| Edit | ||
| </TremorButton> | ||
| </TableCell> | ||
| </TableRow> |
There was a problem hiding this comment.
Edit button sends
null template for non-selected versions
When a user clicks "Edit" directly on a version row that isn't the currently loaded version, raw_prompt_template will be null because isSelected is false (vNum !== selectedVersion). The edit button uses e.stopPropagation(), so the row click handler (handleVersionClick) is not called — the template is never fetched for that version before onEdit is invoked.
This means clicking "Edit" on any version except the one currently displayed will open the editor with an empty/null template, silently discarding the existing prompt content.
The fix is to always fetch the version's info before calling onEdit:
onClick={async (e) => {
e.stopPropagation();
let template = isSelected ? promptTemplate : null;
if (!isSelected) {
try {
const versionedId = `${promptId}.v${vNum}`;
const resp = await getPromptInfo(accessToken, versionedId, selectedEnv || undefined);
template = resp.raw_prompt_template;
} catch {
NotificationsManager.fromBackend(`Failed to load version v${vNum} for editing`);
return;
}
}
const editData = {
prompt_spec: { ...v, prompt_id: basePromptId, environment: selectedEnv },
raw_prompt_template: template,
};
onEdit?.(editData);
}}|
|
||
| if (loading) { | ||
| // When environment changes (user clicks tab), re-fetch — skip initial mount | ||
| useEffect(() => { | ||
| if (isInitialMount.current) { | ||
| isInitialMount.current = false; | ||
| // Still fetch version history on initial mount once selectedEnv is set | ||
| if (selectedEnv && accessToken) { | ||
| fetchVersionHistory(selectedEnv); | ||
| } | ||
| return; | ||
| } | ||
| if (selectedEnv && accessToken) { | ||
| fetchPromptInfo(selectedEnv); | ||
| fetchVersionHistory(selectedEnv); | ||
| } | ||
| }, [selectedEnv]); | ||
|
|
||
| if (loading && !promptData) { | ||
| return <div className="p-4">Loading...</div>; | ||
| } | ||
|
|
There was a problem hiding this comment.
Double
fetchPromptInfo on every initial load
On initial component mount the flow is:
promptId/accessTokeneffect runs →fetchPromptInfo()(no env filter) → response setsselectedEnvviasetSelectedEnv(...).- The
selectedEnveffect fires withselectedEnv = null→isInitialMount.currentis set tofalse, then returns. - React re-renders with the new
selectedEnv→selectedEnveffect fires again. BecauseisInitialMount.currentis alreadyfalseit goes to theelsebranch and callsfetchPromptInfo(selectedEnv)again.
This causes two sequential API calls for every prompt the user opens. Consider tracking whether selectedEnv was set by the initial load vs. a user tab click using a separate ref flag:
const envSetByInitialLoad = useRef(false);
// Inside fetchPromptInfo, after setSelectedEnv:
envSetByInitialLoad.current = true;
setSelectedEnv(...);
// In the selectedEnv effect:
if (envSetByInitialLoad.current) {
envSetByInitialLoad.current = false;
if (selectedEnv && accessToken) fetchVersionHistory(selectedEnv);
return;
}| allowClear | ||
| value={selectedEnvironment} | ||
| onChange={(value) => setSelectedEnvironment(value)} | ||
| style={{ width: 180 }} | ||
| options={[ | ||
| { label: "Development", value: "development" }, | ||
| { label: "Staging", value: "staging" }, | ||
| { label: "Production", value: "production" }, | ||
| ]} |
There was a problem hiding this comment.
Hardcoded environment list in filter dropdown
The environment filter only shows development, staging, and production. Users who define custom environments won't see them in the dropdown and can't filter to them from the UI.
Consider populating the options dynamically from the loaded promptsList:
const environmentOptions = [
...new Set(promptsList.map(p => p.environment).filter(Boolean) as string[])
].sort().map(env => ({ label: env.charAt(0).toUpperCase() + env.slice(1), value: env }));| status_code=403, detail="Only proxy admins can view prompt versions" | ||
| ) | ||
|
|
||
| # Strip version suffix if provided (e.g., "jack_success.v1" -> "jack_success") | ||
| base_prompt_id = get_base_prompt_id(prompt_id=prompt_id) | ||
|
|
||
| # Get all prompts and filter by base_prompt_id | ||
| all_prompts = list(IN_MEMORY_PROMPT_REGISTRY.IN_MEMORY_PROMPTS.values()) | ||
| prompt_versions = [ | ||
| prompt | ||
| for prompt in all_prompts | ||
| if get_base_prompt_id(prompt_id=prompt.prompt_id) == base_prompt_id | ||
| ] | ||
| # Query DB for versions | ||
| versioned_prompts = [] | ||
| if prisma_client is not None: | ||
| where_clause: Dict[str, Any] = {"prompt_id": base_prompt_id} | ||
| if environment: | ||
| where_clause["environment"] = environment | ||
| db_prompts = await prisma_client.db.litellm_prompttable.find_many( | ||
| where=where_clause, | ||
| order={"version": "desc"}, | ||
| ) | ||
| for db_prompt in db_prompts: | ||
| spec = create_versioned_prompt_spec(db_prompt=db_prompt) | ||
| versioned_prompts.append( | ||
| PromptSpec( | ||
| prompt_id=base_prompt_id, | ||
| litellm_params=spec.litellm_params, | ||
| prompt_info=spec.prompt_info, | ||
| created_at=spec.created_at, | ||
| updated_at=spec.updated_at, | ||
| version=get_version_number(prompt_id=spec.prompt_id), | ||
| environment=spec.environment, | ||
| created_by=spec.created_by, | ||
| ) | ||
| ) | ||
| else: | ||
| # Fallback: in-memory registry (no DB) | ||
| all_prompts = list(IN_MEMORY_PROMPT_REGISTRY.IN_MEMORY_PROMPTS.values()) | ||
| prompt_versions = [ | ||
| prompt | ||
| for prompt in all_prompts | ||
| if get_base_prompt_id(prompt_id=prompt.prompt_id) == base_prompt_id | ||
| and (environment is None or prompt.environment == environment) | ||
| ] | ||
| for prompt in prompt_versions: | ||
| version_number = get_version_number(prompt_id=prompt.prompt_id) |
There was a problem hiding this comment.
get_prompt_versions silently returns empty list when prisma_client is None
The new implementation only queries the database. If prisma_client is None (no DB configured), the function returns an empty list rather than falling back to the in-memory registry as the old code did. Similarly, get_prompt_info only falls back to in-memory when environment is None — specifying an environment without a DB yields a 400.
Consider adding an explicit error message explaining that environment-scoped version history requires a database connection, or add a fallback path.
* Litellm ishaan april1 (BerriAI#25103) * fix(proxy): enforce upperbound key params on key/update and add custom_key_update hook The /key/update endpoint did not enforce upperbound_key_generate_params, allowing users to bypass configured limits (tpm_limit, rpm_limit, max_budget, duration, budget_duration) by updating an existing key instead of generating a new one. Extract the upperbound enforcement logic from _common_key_generation_helper() into a standalone _enforce_upperbound_key_params() function and call it from both the generate and update paths. For updates, None values are skipped (not filled with defaults) since they mean "don't change this field". Also adds a custom_key_update config option and user_custom_key_update global, mirroring the existing custom_key_generate pattern, so custom key validation logic can fire during key updates as well. * fix(proxy): invoke custom_key_update hook in bulk update path The user_custom_key_update hook was only called in update_key_fn (single key update) but not in _process_single_key_update (bulk update path), allowing custom validation to be bypassed via the /key/update/bulk endpoint. Mirror the hook invocation in both paths. * fix(proxy): pass UpdateKeyRequest to hook in bulk path, not BulkUpdateKeyRequestItem Move the custom_key_update hook invocation to after UpdateKeyRequest is constructed so the hook receives the same type in both single and bulk update paths. Previously the bulk path passed BulkUpdateKeyRequestItem (5 fields only), which would cause AttributeError for hooks accessing fields like tpm_limit or models. * fix(bedrock): promote cache usage to message_delta for Claude Code (BerriAI#24850) Ensure Bedrock/Anthropic-compatible streaming exposes cache usage where Claude Code reads it by promoting message_stop usage onto message_delta and preserving usage fields in fake-streamed message_delta events. Made-with: Cursor * fix(search): Support self-hosted Firecrawl response format in search transform (BerriAI#24866) The `transform_search_response` method only handled Firecrawl Cloud (v2) response format where `data` is a dict with `web`/`news` keys. Self-hosted Firecrawl (v1) returns `data` as a flat list of result objects, causing an `AttributeError: 'list' object has no attribute 'get'`. Detect the response format by checking if `data` is a list (self-hosted) or dict (cloud) and handle both cases. Cloud format: {"data": {"web": [...], "news": [...]}} Self-hosted: {"success": true, "data": [{"url": "...", "title": "...", ...}]} Co-authored-by: Synergy <synergyoclaw@gmail.com> * feat: add environment and user tracking to prompt management (BerriAI#24855) * feat: add environment and user tracking to prompt management - Add environment (development/staging/production) and created_by columns to LiteLLM_PromptTable - Update unique constraint to [prompt_id, version, environment] - All CRUD endpoints support environment filtering and user tracking - Redesigned prompt detail page with environment tabs and version history - UI: environment filter on list page, environment selector in editor - 8 new tests for environment and user tracking * fix: Black formatting and add environments to PromptInfoResponse TypeScript type * fix: address Greptile review findings - P1: delete_prompt scopes in-memory cleanup to environment when provided - P2: dotprompt_content parsed directly regardless of environment flag - P2: use distinct for environments query - P2: fix double-fetch on initial mount in prompt_info.tsx - fix: remove unsupported select kwarg from find_many * fix: address remaining Greptile review comments - Remove unused useCallback import (index.tsx) - Remove unused ENV_COLORS variable (prompt_info.tsx) - P1: in-memory fallback in get_prompt_versions now respects environment filter - P1: reset selectedEnv when promptId changes to avoid stale state - Cyclic imports are pre-existing pattern, not introduced by this PR * fix: scope patch_prompt to environment using primary key - Add environment query param to patch_prompt endpoint - Look up target row by composite key (prompt_id + version + environment) - Update by primary key (id) to target exactly one row - Fixes Greptile finding: patch with multiple environments no longer ambiguous --------- * fix: use actual start_time for failed request spend logs (BerriAI#24906) async_post_call_failure_hook set both start_time and end_time to datetime.now(), making all failed requests show duration=0. Use the actual start_time from litellm_logging_obj instead, so spend logs reflect the real request duration on timeout and other failures. Fixes BerriAI#24888 * feat(bedrock): add nova canvas image edit support (BerriAI#24869) * feat(bedrock): add nova canvas image edit support * fix(bedrock): support PathLike inputs for nova image edit * chore: sync schema.prisma copies from root * fix(mypy): correct type-ignore code for delta_usage arg-type * fix(mypy): cast status_code to str, suppress intentional str yield * fix(lint): extract _create_content_block_chunks to fix PLR0915 * fix(lint): extract helpers to fix PLR0915 in prompt endpoints --------- Co-authored-by: michelligabriele <gabriele.michelli@icloud.com> Co-authored-by: Sameer Kankute <sameer@berri.ai> Co-authored-by: redhelix <amin.lalji@gmail.com> Co-authored-by: Synergy <synergyoclaw@gmail.com> Co-authored-by: Talha Anwar <37379131+talhaanwarch@users.noreply.github.com> Co-authored-by: madhu19991 <madhu@thunkai.com> Co-authored-by: Srikanth @adobe <devarakondasrikanth@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(test): update model armor streaming test to handle string or int error code --------- Co-authored-by: michelligabriele <gabriele.michelli@icloud.com> Co-authored-by: Sameer Kankute <sameer@berri.ai> Co-authored-by: redhelix <amin.lalji@gmail.com> Co-authored-by: Synergy <synergyoclaw@gmail.com> Co-authored-by: Talha Anwar <37379131+talhaanwarch@users.noreply.github.com> Co-authored-by: madhu19991 <madhu@thunkai.com> Co-authored-by: Srikanth @adobe <devarakondasrikanth@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes