-
Notifications
You must be signed in to change notification settings - Fork 25
add cache-warmup flag #149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a cache-warmup mode to the evaluation runner that clears and recreates configured cache directories before evaluation; documentation and pyproject notes updated; unit tests expanded extensively; a new CLI flag (--cache-warmup) and a private helper Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Main as main()
participant Eval as run_evaluation()
participant ClearC as _clear_caches()
participant FS as Filesystem<br/>(cache dirs)
User->>Main: invoke CLI with --cache-warmup
Main->>Eval: call run_evaluation(eval_args{cache_warmup: true})
Eval->>Eval: detect cache_warmup == True
Eval->>Eval: print "warming caches" message
Eval->>ClearC: call _clear_caches(system_config)
ClearC->>ClearC: enumerate enabled caches from SystemConfig
ClearC->>FS: remove existing cache directories
ClearC->>FS: recreate empty cache directories
ClearC-->>Eval: return
Eval->>Eval: continue loading heavy modules and run evaluation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/lightspeed_evaluation/runner/evaluation.py`:
- Around line 95-99: Replace the print calls in run_evaluation (the cache-warmup
branch using eval_args.cache_warmup) and inside _clear_caches with structured
logging: after configuration loads call setup_logging() from
system_config.logging to get a logger instance, then use logger.info(...)
instead of print(...) for the "Cache warmup mode" message and any output
produced by _clear_caches so messages follow the codebase logging conventions
and levels.
- Around line 17-46: The _clear_caches function must validate each cache_dir
before calling shutil.rmtree to avoid deleting high-level or unexpected
locations; for each (cache_name, cache_dir) in cache_dirs, resolve the path with
Path(cache_dir).resolve() and ensure it is not root, not equal to Path.cwd(),
and is a subdirectory of an allowed base (e.g., contains a trusted segment like
".caches" or matches a configured base cache directory); if any check fails
raise DataValidationError with a clear message referencing cache_name and
cache_dir, otherwise proceed to shutil.rmtree and recreate the directory as
before (keep references to shutil.rmtree, Path.mkdir, cache_dirs, and
_clear_caches when locating code).
| # Clear caches if cache warmup mode is enabled | ||
| if eval_args.cache_warmup: | ||
| print("\n🔥 Cache warmup mode: Clearing existing caches...") | ||
| _clear_caches(system_config) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how logging is set up and used in the runner to align with existing patterns
rg -n "setup_logging|logging\.getLogger|logger\." src/lightspeed_evaluation/runner -g '*.py' -A 2Repository: lightspeed-core/lightspeed-evaluation
Length of output: 63
🏁 Script executed:
#!/bin/bash
# First, check the file size and then read it
wc -l src/lightspeed_evaluation/runner/evaluation.pyRepository: lightspeed-core/lightspeed-evaluation
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Read the evaluation.py file to understand logging setup and the context of lines 95-99
cat -n src/lightspeed_evaluation/runner/evaluation.pyRepository: lightspeed-core/lightspeed-evaluation
Length of output: 9300
🏁 Script executed:
#!/bin/bash
# Search for logging patterns in the broader codebase
rg -n "import logging|logging\.getLogger" src/lightspeed_evaluation -g '*.py' | head -20Repository: lightspeed-core/lightspeed-evaluation
Length of output: 1811
🏁 Script executed:
#!/bin/bash
# Check what log levels are used in similar runner/initialization contexts
rg -n "logger\.(info|debug|warning)" src/lightspeed_evaluation/pipeline/evaluation/pipeline.py src/lightspeed_evaluation/core/system/loader.py -A 1 | head -30Repository: lightspeed-core/lightspeed-evaluation
Length of output: 2355
🏁 Script executed:
#!/bin/bash
# Verify the logging setup is available in the loader module
rg -n "logger\s*=" src/lightspeed_evaluation/core/system/loader.py -A 2 | head -10Repository: lightspeed-core/lightspeed-evaluation
Length of output: 192
🏁 Script executed:
#!/bin/bash
# Check the setup_logging function to understand how to initialize logging in evaluation.py
rg -n "def setup_logging" src/lightspeed_evaluation -A 10Repository: lightspeed-core/lightspeed-evaluation
Length of output: 1023
Replace print statements with structured logging.
The cache-warmup messaging at lines 97-98 and all output in _clear_caches use print, which bypasses the configured logger. Update to use structured logging at INFO level throughout run_evaluation and _clear_caches to align with established patterns in the codebase (e.g., pipeline.py, processor.py) and the coding guideline requiring structured logging with appropriate log levels. Initialize the logger using setup_logging() from system_config.logging after configuration loads.
🤖 Prompt for AI Agents
In `@src/lightspeed_evaluation/runner/evaluation.py` around lines 95 - 99, Replace
the print calls in run_evaluation (the cache-warmup branch using
eval_args.cache_warmup) and inside _clear_caches with structured logging: after
configuration loads call setup_logging() from system_config.logging to get a
logger instance, then use logger.info(...) instead of print(...) for the "Cache
warmup mode" message and any output produced by _clear_caches so messages follow
the codebase logging conventions and levels.
Description
The PR implements
--cache-warmupflag that clears the caches at the startup.Type of change
Tools used to create PR
Related Tickets & Documents
Checklist before requesting a review
Summary by CodeRabbit
New Features
Documentation
Dependencies
Tests
✏️ Tip: You can customize this high-level summary in your review settings.