fix(dynamic_rate_limiter): make test_priority_reservation deterministic across minute rollover

[mateo-berri]

Relevant issues

Linear ticket

Pre-Submission checklist

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

• I have added meaningful tests
• My PR passes all CI/CD checks (e.g., lint, format, unit tests)
• 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

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

test_priority_reservation[100] failed on CircleCI job 2000264 with assert 90 == 0. DynamicRateLimiterCache keys the active-project set by the current UTC minute ("%H-%M:{model}"), and the test's async_set_cache_sadd and the handler's read inside check_available_usage each call get_utc_datetime() independently. When the wall-clock minute rolls over between the two calls, the read targets the next minute's empty key, active_projects comes back None, the divide-by-projects step is skipped, and availability is int(100 * 0.9) = 90 for every parametrization; only [1] survives that because its expected value happens to also be 90. The sibling test_multiple_projects_e2e was skipped back in June 2024 for this exact class of problem ("Unstable on ci/cd due to curr minute changes")

Deterministic reproduction of the mechanism on the parent commit, driving the two clock reads across a minute boundary:

$ python /tmp/repro_minute_rollover.py
sadd at 23:04:59.999, get at 23:05:00.001 -> availability: 90 | expected: 0

Plain rerunning cannot catch it because the race window is sub-millisecond (20 of 20 green before the fix on a quiet clock):

$ cd tests/local_testing && for i in $(seq 1 20); do LITELLM_LICENSE=... python -m pytest "test_dynamic_rate_limit_handler.py::test_priority_reservation" -q -p no:cacheprovider >/dev/null 2>&1 && echo "run $i: pass" || echo "run $i: FAIL"; done
run 1: pass ... run 20: pass   (pre-fix:  20 passed, 0 failed; flake needs a minute rollover)
run 1: pass ... run 20: pass   (post-fix: 20 passed, 0 failed; clock frozen, rollover impossible)

With the fix, the fixture pins the limiter's clock, so the sadd and the read can never straddle a minute boundary, and tests/test_litellm/proxy/hooks/test_dynamic_rate_limiter.py::test_minute_rollover_between_sadd_and_get_reads_empty_window pins the rollover mechanism itself deterministically

Type

🐛 Bug Fix
✅ Test

Changes

DynamicRateLimiterCache and _PROXY_DynamicRateLimitHandler now take an injectable time_fn (defaulting to get_utc_datetime, so production behavior is unchanged) instead of hardcoding the clock; this follows the repo preference for dependency injection over monkeypatching. The flaky tests/local_testing/test_dynamic_rate_limit_handler.py fixture injects a frozen clock, which makes test_priority_reservation (and the other tests in that file that share the same write-then-read-within-one-window assumption, e.g. test_update_cache, whose 2s sleep gave it roughly a 1 in 30 chance of straddling a rollover per run) deterministic. New unit tests in tests/test_litellm/proxy/hooks/test_dynamic_rate_limiter.py cover the injected-clock key window, the minute-rollover behavior that caused the flake, and the handler threading time_fn through to its cache

[mateo-berri]

@greptileai

[greptile-apps]

Greptile Summary

This PR makes DynamicRateLimiterCache and _PROXY_DynamicRateLimitHandler accept an injectable time_fn clock (defaulting to get_utc_datetime), then freezes that clock in the existing test fixture to eliminate the minute-rollover race that caused test_priority_reservation to produce assert 90 == 0 on CI.

• Production hook (dynamic_rate_limiter.py): two get_utc_datetime() call-sites replaced with self.time_fn(); default keeps the original behavior with no user-visible change.
• Existing test fixture (test_dynamic_rate_limit_handler.py): injects datetime(2024, 1, 1, 10, 30, 0, UTC) so the sadd and get always target the same minute key, making all tests in the file deterministic.
• New unit tests (test_dynamic_rate_limiter.py): three async tests cover the frozen-clock key-sharing, the rollover-to-empty-window mechanism, and the handler-to-cache threading — all using only local in-memory cache.

Confidence Score: 5/5

The change is minimal and surgical: a two-line replacement in production code with a well-defaulted parameter, plus a fixture update and new focused unit tests. No user-facing behavior changes.

The production change is a textbook dependency-injection seam with a sensible default. The existing tests are made more deterministic, not weakened. The new tests deliberately cover the exact failure scenario described in the PR description. Nothing in the critical request path is structurally altered.

No files require special attention. The two still-skipped tests (test_multiple_projects, test_multiple_projects_e2e) are a pre-existing concern noted in a prior review comment.

Important Files Changed

Filename	Overview
litellm/proxy/hooks/dynamic_rate_limiter.py	Adds injectable time_fn: Callable[[], datetime] (defaulting to get_utc_datetime) to DynamicRateLimiterCache and _PROXY_DynamicRateLimitHandler; replaces two hardcoded get_utc_datetime() calls with self.time_fn() — production behavior is unchanged, tests gain a clock seam
tests/local_testing/test_dynamic_rate_limit_handler.py	Fixture now injects a frozen UTC clock into DynamicRateLimitHandler, eliminating the minute-rollover race that caused test_priority_reservation to flake on CI; two skip-decorated tests remain that would now be safe to un-skip
tests/test_litellm/proxy/hooks/test_dynamic_rate_limiter.py	New unit-test file covering three scenarios: frozen-clock sadd/get share the same key, minute-rollover between sadd and get returns None, and the handler correctly threads time_fn to its inner cache — no real network calls

_{Reviews (3): Last reviewed commit: "fix(dynamic_rate_limiter): inject clock ..." | Re-trigger Greptile}

[greptile-apps]

Greptile Summary

This PR fixes a flaky test (test_priority_reservation) caused by a minute-rollover race condition in DynamicRateLimiterCache. The cache keys active projects by UTC minute, and a wall-clock rollover between a test's async_set_cache_sadd write and the handler's subsequent async_get_cache read would target an empty key, causing active_projects to come back None and the per-project quota division to be skipped — producing int(100 * 0.9) = 90 instead of the expected 0.

• Adds an injectable time_fn: Callable[[], datetime] (defaulting to get_utc_datetime) to DynamicRateLimiterCache and _PROXY_DynamicRateLimitHandler, following the repo's dependency-injection preference; production behavior is unchanged.
• The existing test fixture now injects a frozen clock so all writes and reads in a single test always use the same minute key, eliminating the race.
• Three new focused unit tests in tests/test_litellm/proxy/hooks/test_dynamic_rate_limiter.py cover the injected-clock key window, the rollover mechanism itself, and handler-to-cache time_fn threading.

Confidence Score: 5/5

Safe to merge. The change is a minimal, additive dependency injection with a backwards-compatible default that leaves all production call paths identical to before.

The production change is a single optional parameter threaded through two constructors; the default value preserves existing behaviour exactly. The test fixture change pins the clock rather than altering any assertion, and the new tests are focused, self-contained, and cover the specific failure mode that caused the flake. No logic in the request path is altered.

No files require special attention. The two long-running tests (test_multiple_projects, test_multiple_projects_e2e) remain skipped — the frozen clock fix would stabilise them too, but that is out of scope for this PR.

Important Files Changed

Filename	Overview
litellm/proxy/hooks/dynamic_rate_limiter.py	Adds optional time_fn injectable clock to DynamicRateLimiterCache and _PROXY_DynamicRateLimitHandler; all get_utc_datetime() call sites replaced with self.time_fn(); production default unchanged.
tests/local_testing/test_dynamic_rate_limit_handler.py	Fixture updated to inject a frozen UTC clock, making all write-then-read tests deterministic against minute rollover; no test assertions weakened.
tests/test_litellm/proxy/hooks/test_dynamic_rate_limiter.py	New unit tests covering shared clock window, minute-rollover behavior, and handler-to-cache time_fn threading; all use in-memory DualCache with no network calls.

_{Reviews (2): Last reviewed commit: "fix(dynamic_rate_limiter): inject clock ..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[mateo-berri]

test_multiple_projects (line 345) and test_multiple_projects_e2e (line 495)

those tests also depend on the router's own un-injected minute clock (router.py:8890) plus 15s of real sleeps, so re-enabling them would still flake ~1 in 3 runs

[mateo-berri]

@greptileai