mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(budget): resolve word-form budget_duration so it no longer silently resets daily (#34250)
* fix(budget): resolve word-form budget_duration so it no longer silently resets daily Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(budget): normalize legacy word-form budget_duration on key edit load so untouched saves stay canonical Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(budget): preserve canonical budget_duration in key update submit handler Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: milan <milan@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
abf18f8760
commit
eb2dce8771
6 changed files with 224 additions and 20 deletions
|
|
@ -9,9 +9,22 @@ duration_in_seconds is used in diff parts of the code base, example
|
|||
import re
|
||||
import time as time_module
|
||||
from datetime import datetime, time, timedelta, timezone, tzinfo
|
||||
from typing import Optional, Tuple
|
||||
from typing import Final, Optional, Tuple
|
||||
from zoneinfo import ZoneInfo
|
||||
|
||||
from litellm._logging import verbose_logger
|
||||
|
||||
_BUDGET_DURATION_WORD_ALIASES: Final[dict[str, str]] = {
|
||||
"hourly": "1h",
|
||||
"daily": "24h",
|
||||
"weekly": "7d",
|
||||
"monthly": "30d",
|
||||
}
|
||||
|
||||
|
||||
def _normalize_duration(duration: str) -> str:
|
||||
return _BUDGET_DURATION_WORD_ALIASES.get(duration.strip().lower(), duration)
|
||||
|
||||
|
||||
def _extract_from_regex(duration: str) -> Tuple[int, str]:
|
||||
match = re.match(r"(\d+)(mo|[smhdw]?)", duration)
|
||||
|
|
@ -48,7 +61,7 @@ def duration_in_seconds(duration: str) -> int:
|
|||
|
||||
Returns time in seconds till when budget needs to be reset
|
||||
"""
|
||||
value, unit = _extract_from_regex(duration=duration)
|
||||
value, unit = _extract_from_regex(duration=_normalize_duration(duration))
|
||||
|
||||
if unit == "s":
|
||||
return value
|
||||
|
|
@ -124,9 +137,13 @@ def get_next_standardized_reset_time(
|
|||
current_time, _ = _setup_timezone(current_time, timezone_str)
|
||||
|
||||
# Parse duration
|
||||
value, unit = _parse_duration(duration)
|
||||
value, unit = _parse_duration(_normalize_duration(duration))
|
||||
if value is None:
|
||||
# Fall back to default if format is invalid
|
||||
verbose_logger.warning(
|
||||
"Unrecognized budget_duration %r; falling back to a next-midnight reset. "
|
||||
"Use the <int><unit> format (e.g. '1h', '7d', '30d', '1mo').",
|
||||
duration,
|
||||
)
|
||||
return current_time.replace(hour=0, minute=0, second=0, microsecond=0) + timedelta(days=1)
|
||||
|
||||
# Midnight of the current day in the specified timezone
|
||||
|
|
|
|||
|
|
@ -1,8 +1,13 @@
|
|||
import unittest
|
||||
from datetime import datetime, time, timezone
|
||||
from unittest.mock import patch
|
||||
from zoneinfo import ZoneInfo
|
||||
|
||||
from litellm.litellm_core_utils.duration_parser import get_next_standardized_reset_time
|
||||
import litellm.litellm_core_utils.duration_parser as duration_parser
|
||||
from litellm.litellm_core_utils.duration_parser import (
|
||||
duration_in_seconds,
|
||||
get_next_standardized_reset_time,
|
||||
)
|
||||
|
||||
|
||||
class TestStandardizedResetTime(unittest.TestCase):
|
||||
|
|
@ -316,5 +321,69 @@ class TestResetTimeOfDay(unittest.TestCase):
|
|||
)
|
||||
|
||||
|
||||
class TestWordFormBudgetDurations(unittest.TestCase):
|
||||
"""The Admin UI historically persisted word-form budget durations
|
||||
(hourly/daily/weekly/monthly). They must resolve to their real interval
|
||||
instead of silently collapsing to a next-midnight (daily) reset.
|
||||
"""
|
||||
|
||||
def test_word_forms_map_to_correct_reset_times(self):
|
||||
base_time = datetime(2023, 5, 17, 15, 20, 30, tzinfo=timezone.utc)
|
||||
|
||||
self.assertEqual(
|
||||
get_next_standardized_reset_time("hourly", base_time, "UTC"),
|
||||
datetime(2023, 5, 17, 16, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
self.assertEqual(
|
||||
get_next_standardized_reset_time("daily", base_time, "UTC"),
|
||||
datetime(2023, 5, 18, 0, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
self.assertEqual(
|
||||
get_next_standardized_reset_time("weekly", base_time, "UTC"),
|
||||
datetime(2023, 5, 22, 0, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
self.assertEqual(
|
||||
get_next_standardized_reset_time("monthly", base_time, "UTC"),
|
||||
datetime(2023, 6, 1, 0, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
|
||||
def test_word_forms_are_not_all_collapsed_to_daily(self):
|
||||
base_time = datetime(2023, 5, 17, 15, 20, 30, tzinfo=timezone.utc)
|
||||
results = {
|
||||
word: get_next_standardized_reset_time(word, base_time, "UTC")
|
||||
for word in ("hourly", "daily", "weekly", "monthly")
|
||||
}
|
||||
self.assertEqual(len(set(results.values())), len(results))
|
||||
|
||||
def test_word_forms_match_canonical_int_unit_forms(self):
|
||||
base_time = datetime(2023, 5, 17, 15, 20, 30, tzinfo=timezone.utc)
|
||||
for word, canonical in (("hourly", "1h"), ("daily", "24h"), ("weekly", "7d"), ("monthly", "30d")):
|
||||
self.assertEqual(
|
||||
get_next_standardized_reset_time(word, base_time, "UTC"),
|
||||
get_next_standardized_reset_time(canonical, base_time, "UTC"),
|
||||
)
|
||||
|
||||
def test_word_forms_are_case_and_whitespace_insensitive(self):
|
||||
base_time = datetime(2023, 5, 17, 15, 20, 30, tzinfo=timezone.utc)
|
||||
self.assertEqual(
|
||||
get_next_standardized_reset_time(" Monthly ", base_time, "UTC"),
|
||||
datetime(2023, 6, 1, 0, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
|
||||
def test_duration_in_seconds_accepts_word_forms(self):
|
||||
self.assertEqual(duration_in_seconds("hourly"), 3600)
|
||||
self.assertEqual(duration_in_seconds("daily"), 86400)
|
||||
self.assertEqual(duration_in_seconds("weekly"), 604800)
|
||||
self.assertEqual(duration_in_seconds("monthly"), 2592000)
|
||||
|
||||
def test_invalid_duration_logs_warning_and_falls_back(self):
|
||||
base_time = datetime(2023, 5, 15, 15, 0, 0, tzinfo=timezone.utc)
|
||||
with patch.object(duration_parser.verbose_logger, "warning") as mock_warning:
|
||||
result = get_next_standardized_reset_time("garbage", base_time, "UTC")
|
||||
self.assertEqual(result, datetime(2023, 5, 16, 0, 0, 0, tzinfo=timezone.utc))
|
||||
mock_warning.assert_called_once()
|
||||
self.assertIn("garbage", mock_warning.call_args.args)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
|
|
|||
|
|
@ -453,6 +453,44 @@ describe("KeyInfoView handleKeyUpdate guardrails guard", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("KeyInfoView handleKeyUpdate budget_duration", () => {
|
||||
it("should send a canonical budget_duration through unchanged", async () => {
|
||||
renderView(true);
|
||||
|
||||
fireEvent.click(screen.getByText("Settings"));
|
||||
fireEvent.click(screen.getByText("Edit Settings"));
|
||||
(globalThis as any).__TEST_FORM_VALUES = {
|
||||
token: "tok_123",
|
||||
budget_duration: "30d",
|
||||
};
|
||||
|
||||
fireEvent.click(screen.getByText("Mock Submit"));
|
||||
|
||||
await waitFor(() => expect(keyUpdateCallMock).toHaveBeenCalled());
|
||||
|
||||
const [, sentPayload] = keyUpdateCallMock.mock.calls[0];
|
||||
expect(sentPayload.budget_duration).toBe("30d");
|
||||
});
|
||||
|
||||
it("should heal a legacy word-form budget_duration to canonical", async () => {
|
||||
renderView(true);
|
||||
|
||||
fireEvent.click(screen.getByText("Settings"));
|
||||
fireEvent.click(screen.getByText("Edit Settings"));
|
||||
(globalThis as any).__TEST_FORM_VALUES = {
|
||||
token: "tok_123",
|
||||
budget_duration: "monthly",
|
||||
};
|
||||
|
||||
fireEvent.click(screen.getByText("Mock Submit"));
|
||||
|
||||
await waitFor(() => expect(keyUpdateCallMock).toHaveBeenCalled());
|
||||
|
||||
const [, sentPayload] = keyUpdateCallMock.mock.calls[0];
|
||||
expect(sentPayload.budget_duration).toBe("30d");
|
||||
});
|
||||
});
|
||||
|
||||
describe("KeyInfoView handleKeyUpdate empty strings", () => {
|
||||
["tpm_limit", "rpm_limit", "max_parallel_requests", "max_budget"].forEach((limit) => {
|
||||
it(`maps empty strings to null for ${limit}`, async () => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { fireEvent, screen, waitFor } from "@testing-library/react";
|
||||
import { fireEvent, screen, waitFor, within } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { renderWithProviders } from "../../../tests/test-utils";
|
||||
|
|
@ -662,6 +662,87 @@ describe("KeyEditView", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("should persist a canonical budget_duration value, not a word-form the backend cannot parse", async () => {
|
||||
const onSubmitMock = vi.fn().mockResolvedValue(undefined);
|
||||
renderWithProviders(
|
||||
<KeyEditView
|
||||
keyData={MOCK_KEY_DATA}
|
||||
onCancel={() => {}}
|
||||
onSubmit={onSubmitMock}
|
||||
accessToken={"test-token"}
|
||||
userID={"test-user"}
|
||||
userRole={"admin"}
|
||||
premiumUser={false}
|
||||
/>,
|
||||
);
|
||||
|
||||
const resetBudgetItem = (await screen.findByText("Reset Budget")).closest(".ant-form-item");
|
||||
expect(resetBudgetItem).not.toBeNull();
|
||||
const combobox = within(resetBudgetItem as HTMLElement).getByRole("combobox");
|
||||
await userEvent.click(combobox);
|
||||
|
||||
const weeklyOption = await screen.findByText("weekly");
|
||||
await userEvent.click(weeklyOption);
|
||||
|
||||
const submitButton = screen.getByRole("button", { name: /save changes/i });
|
||||
await userEvent.click(submitButton);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onSubmitMock).toHaveBeenCalled();
|
||||
const callArgs = onSubmitMock.mock.calls[0][0];
|
||||
expect(callArgs.budget_duration).toBe("7d");
|
||||
});
|
||||
});
|
||||
|
||||
it("should keep an existing canonical budget_duration canonical when saved untouched", async () => {
|
||||
const onSubmitMock = vi.fn().mockResolvedValue(undefined);
|
||||
renderWithProviders(
|
||||
<KeyEditView
|
||||
keyData={MOCK_KEY_DATA}
|
||||
onCancel={() => {}}
|
||||
onSubmit={onSubmitMock}
|
||||
accessToken={"test-token"}
|
||||
userID={"test-user"}
|
||||
userRole={"admin"}
|
||||
premiumUser={false}
|
||||
/>,
|
||||
);
|
||||
|
||||
const submitButton = await screen.findByRole("button", { name: /save changes/i });
|
||||
await userEvent.click(submitButton);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onSubmitMock).toHaveBeenCalled();
|
||||
const callArgs = onSubmitMock.mock.calls[0][0];
|
||||
expect(callArgs.budget_duration).toBe("30d");
|
||||
});
|
||||
});
|
||||
|
||||
it("should heal a legacy word-form budget_duration to canonical when saved untouched", async () => {
|
||||
const onSubmitMock = vi.fn().mockResolvedValue(undefined);
|
||||
const legacyKeyData = { ...MOCK_KEY_DATA, budget_duration: "monthly" };
|
||||
renderWithProviders(
|
||||
<KeyEditView
|
||||
keyData={legacyKeyData}
|
||||
onCancel={() => {}}
|
||||
onSubmit={onSubmitMock}
|
||||
accessToken={"test-token"}
|
||||
userID={"test-user"}
|
||||
userRole={"admin"}
|
||||
premiumUser={false}
|
||||
/>,
|
||||
);
|
||||
|
||||
const submitButton = await screen.findByRole("button", { name: /save changes/i });
|
||||
await userEvent.click(submitButton);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onSubmitMock).toHaveBeenCalled();
|
||||
const callArgs = onSubmitMock.mock.calls[0][0];
|
||||
expect(callArgs.budget_duration).toBe("30d");
|
||||
});
|
||||
});
|
||||
|
||||
it("should omit budget_limits when existing windows are left untouched (issue #33246)", async () => {
|
||||
// The backend treats any budget_limits in the payload as an admin-only
|
||||
// budget change, so re-sending untouched windows 403s a non-admin owner.
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import { useEffect, useState } from "react";
|
|||
import { rolesWithWriteAccess } from "../../utils/roles";
|
||||
import AgentSelector from "../agent_management/AgentSelector";
|
||||
import AccessGroupSelector from "../common_components/AccessGroupSelector";
|
||||
import BudgetDurationDropdown from "../common_components/budget_duration_dropdown";
|
||||
import { mapInternalToDisplayNames } from "../callback_info_helpers";
|
||||
import KeyLifecycleSettings from "../common_components/KeyLifecycleSettings";
|
||||
import PassThroughRoutesSelector from "../common_components/PassThroughRoutesSelector";
|
||||
|
|
@ -172,15 +173,16 @@ export function KeyEditView({
|
|||
form.setFieldValue("disabled_callbacks", disabledCallbacks);
|
||||
}, [form, disabledCallbacks]);
|
||||
|
||||
// Convert API budget duration to form format
|
||||
// Normalize any legacy word-form budget duration to the canonical value the dropdown uses
|
||||
const getBudgetDuration = (duration: string | null) => {
|
||||
if (!duration) return null;
|
||||
const durationMap: Record<string, string> = {
|
||||
"24h": "daily",
|
||||
"7d": "weekly",
|
||||
"30d": "monthly",
|
||||
const wordToCanonical: Record<string, string> = {
|
||||
hourly: "1h",
|
||||
daily: "24h",
|
||||
weekly: "7d",
|
||||
monthly: "30d",
|
||||
};
|
||||
return durationMap[duration] || null;
|
||||
return wordToCanonical[duration] ?? duration;
|
||||
};
|
||||
|
||||
// Set initial form values
|
||||
|
|
@ -506,11 +508,7 @@ export function KeyEditView({
|
|||
</Form.Item>
|
||||
|
||||
<Form.Item label="Reset Budget" name="budget_duration">
|
||||
<Select placeholder="n/a">
|
||||
<Select.Option value="daily">Daily</Select.Option>
|
||||
<Select.Option value="weekly">Weekly</Select.Option>
|
||||
<Select.Option value="monthly">Monthly</Select.Option>
|
||||
</Select>
|
||||
<BudgetDurationDropdown />
|
||||
</Form.Item>
|
||||
|
||||
<Form.Item
|
||||
|
|
|
|||
|
|
@ -295,14 +295,15 @@ export default function KeyInfoView({
|
|||
}
|
||||
delete formValues.logging_settings;
|
||||
|
||||
// Convert budget_duration to API format
|
||||
// Normalize any legacy word-form budget_duration to the canonical API format
|
||||
if (formValues.budget_duration) {
|
||||
const durationMap: Record<string, string> = {
|
||||
const wordToCanonical: Record<string, string> = {
|
||||
hourly: "1h",
|
||||
daily: "24h",
|
||||
weekly: "7d",
|
||||
monthly: "30d",
|
||||
};
|
||||
formValues.budget_duration = durationMap[formValues.budget_duration];
|
||||
formValues.budget_duration = wordToCanonical[formValues.budget_duration] ?? formValues.budget_duration;
|
||||
}
|
||||
|
||||
const newKeyValues = await keyUpdateCall(accessToken, formValues);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue