Fix Redis key generation to be stable across working directories - #27025
Conversation
`os.path.relpath` with no `start` arg uses the current working directory, so running pytest from a subdirectory produced a different Redis key than running from the repo root. CI-recorded cassettes and locally-replayed runs would silently miss each other's cache. Anchor the path to the repo root (derived from `__file__`) so the key is stable regardless of CWD. https://claude.ai/code/session_018uCx7pcrkdUJZrCVMaTdPx
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes Confidence Score: 4/5Safe to merge; fix is correct for all practical cases and only has minor hardening gaps. The fix is logically sound and well-tested. The only findings are P2: a basename-based fallback that could theoretically collide on Windows cross-drive scenarios, and use of tests/_vcr_redis_persister.py — the fallback and symlink handling in
|
| Filename | Overview |
|---|---|
| tests/_vcr_redis_persister.py | Adds _REPO_ROOT constant computed from __file__ at import time; redis_key_for() now resolves paths to absolute first, then relativizes against _REPO_ROOT, with a ValueError fallback to basename for cross-drive Windows paths. |
| tests/llm_translation/test_vcr_redis_persister.py | Adds test_redis_key_is_stable_across_working_directories which verifies key stability across three different working directories using monkeypatch.chdir; no network calls, correctly uses fakeredis for all other tests. |
Reviews (1): Last reviewed commit: "fix(tests): anchor VCR redis cassette ke..." | Re-trigger Greptile
| VCR_VERBOSE_ENV = "LITELLM_VCR_VERBOSE" | ||
| MAX_EPISODES_PER_CASSETTE = 50 | ||
|
|
||
| _REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) |
There was a problem hiding this comment.
os.path.abspath does not resolve symlinks, so _REPO_ROOT and a cassette path can represent the same physical location via different string prefixes (one through a symlink, one through the real path). In that case os.path.relpath(abs_path, start=_REPO_ROOT) produces a ../../… path which the subsequent lstrip("./") then silently mangles into a wrong key. Using os.path.realpath for both would make the prefix comparison reliable.
| _REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) | |
| _REPO_ROOT = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) |
|
|
||
| def redis_key_for(cassette_path: str) -> str: | ||
| rel = os.path.relpath(str(cassette_path)) | ||
| abs_path = os.path.abspath(str(cassette_path)) |
There was a problem hiding this comment.
Consistent with using
realpath for _REPO_ROOT, abs_path should also be resolved through any symlinks before computing the relative path, otherwise the prefix comparison can still fail when the cassette path arrives via a symlinked directory.
| abs_path = os.path.abspath(str(cassette_path)) | |
| abs_path = os.path.realpath(str(cassette_path)) |
| except ValueError: | ||
| rel = os.path.basename(abs_path) |
There was a problem hiding this comment.
The
ValueError fallback (Windows cross-drive paths) uses only os.path.basename, which drops all directory context. Two cassettes in different subdirectories that share the same filename — e.g. tests/a/test_foo.yaml and tests/b/test_foo.yaml — would both resolve to litellm:vcr:cassette:test_foo and silently share the same Redis entry. Using a short hash of the full absolute path as the fallback would avoid this collision while remaining stable.
| except ValueError: | |
| rel = os.path.basename(abs_path) | |
| except ValueError: | |
| import hashlib | |
| rel = hashlib.sha1(abs_path.encode()).hexdigest()[:16] |
…eneration-d88e Fix Redis key generation to be stable across working directories
Relevant issues
Pre-Submission checklist
Type
🐛 Bug Fix
✅ Test
Changes
Problem
The
redis_key_for()function was generating different Redis keys for the same cassette file depending on the current working directory. This caused cache misses and inconsistent behavior when tests were run from different directories.Solution
Modified
redis_key_for()intests/_vcr_redis_persister.pyto:_REPO_ROOT) instead of the current working directoryTesting
Added
test_redis_key_is_stable_across_working_directories()intests/llm_translation/test_vcr_redis_persister.pythat:litellm:vcr:cassette:tests/llm_translation/test_anthropic/test_streamingThis ensures VCR cassette caching works reliably regardless of where tests are executed from.
Slack Thread