fix: azure env variable names for judgeLLM#129
Conversation
WalkthroughStandardizes Azure environment variable naming across the codebase by renaming Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lsc_agent_eval/README.md (1)
193-193: Document AZURE_API_VERSION if required.The line mentions
AZURE_API_BASE(with a placeholder value), but earlier context in the codebase and Azure OpenAI documentation often reference an optionalAZURE_API_VERSIONvariable. If this variable is used by the LiteLLM integration for Azure, consider documenting it here for completeness.README.md (1)
337-342: LGTM! Consider adding a migration note for existing users.The Azure environment variable documentation is clear and accurate. Since this is a breaking change from previous
AZURE_OPENAI_*variables, consider adding a note in release notes or a migration guide to help existing users update their configurations.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.mdlsc_agent_eval/README.mdlsc_agent_eval/src/lsc_agent_eval/core/utils/judge.pylsc_agent_eval/tests/core/utils/test_judge.pysrc/lightspeed_evaluation/core/llm/manager.pysrc/lightspeed_evaluation/core/system/env_validator.pytests/unit/core/llm/test_llm_manager.pytests/unit/core/system/test_env_validator.py
🧰 Additional context used
📓 Path-based instructions (3)
**/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest mocking (mocker fixture) instead of unittest.mock
Files:
lsc_agent_eval/tests/core/utils/test_judge.pytests/unit/core/llm/test_llm_manager.pytests/unit/core/system/test_env_validator.py
src/lightspeed_evaluation/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/lightspeed_evaluation/**/*.py: Provide type hints for all public functions and methods
Use Google-style docstrings for all public APIs
Use custom exceptions from core.system.exceptions for error handling
Use structured logging with appropriate levels in lightspeed_evaluation modules
Files:
src/lightspeed_evaluation/core/system/env_validator.pysrc/lightspeed_evaluation/core/llm/manager.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Mirror test structure to source code organization, with test files named test_.py and test functions/classes named test_ and Test*
Files:
tests/unit/core/llm/test_llm_manager.pytests/unit/core/system/test_env_validator.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: asamal4
Repo: lightspeed-core/lightspeed-evaluation PR: 47
File: src/lightspeed_evaluation/core/api/__init__.py:3-3
Timestamp: 2025-09-09T08:08:40.654Z
Learning: API_KEY and OPENAI_API_KEY are distinct environment variables with different purposes in the lightspeed-evaluation codebase - they should not be treated as interchangeable fallbacks.
Learnt from: asamal4
Repo: lightspeed-core/lightspeed-evaluation PR: 47
File: src/lightspeed_evaluation/core/output/generator.py:140-145
Timestamp: 2025-09-11T12:47:06.747Z
Learning: User asamal4 prefers that non-critical comments are sent when actual code changes are pushed, not on unrelated commits.
📚 Learning: 2025-12-11T10:05:06.422Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-evaluation PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T10:05:06.422Z
Learning: Do not add new features to lsc_agent_eval/ directory - add to src/lightspeed_evaluation/ instead
Applied to files:
lsc_agent_eval/README.md
📚 Learning: 2025-09-09T08:08:40.654Z
Learnt from: asamal4
Repo: lightspeed-core/lightspeed-evaluation PR: 47
File: src/lightspeed_evaluation/core/api/__init__.py:3-3
Timestamp: 2025-09-09T08:08:40.654Z
Learning: API_KEY and OPENAI_API_KEY are distinct environment variables with different purposes in the lightspeed-evaluation codebase - they should not be treated as interchangeable fallbacks.
Applied to files:
lsc_agent_eval/README.mdlsc_agent_eval/tests/core/utils/test_judge.pysrc/lightspeed_evaluation/core/system/env_validator.pytests/unit/core/system/test_env_validator.pyREADME.mdlsc_agent_eval/src/lsc_agent_eval/core/utils/judge.py
📚 Learning: 2025-09-18T23:59:37.026Z
Learnt from: asamal4
Repo: lightspeed-core/lightspeed-evaluation PR: 55
File: src/lightspeed_evaluation/core/system/validator.py:146-155
Timestamp: 2025-09-18T23:59:37.026Z
Learning: In the lightspeed-evaluation project, the DataValidator in `src/lightspeed_evaluation/core/system/validator.py` is intentionally designed to validate only explicitly provided user evaluation data, not resolved metrics that include system defaults. When turn_metrics is None, the system falls back to system config defaults, and this validation separation is by design.
Applied to files:
src/lightspeed_evaluation/core/system/env_validator.py
📚 Learning: 2025-07-16T12:07:29.169Z
Learnt from: asamal4
Repo: lightspeed-core/lightspeed-evaluation PR: 19
File: lsc_agent_eval/tests/core/agent_goal_eval/test_script_runner.py:0-0
Timestamp: 2025-07-16T12:07:29.169Z
Learning: In the lsc_agent_eval package, the ScriptRunner class was modified to use absolute paths internally rather than documenting path normalization behavior, providing more predictable and consistent path handling.
Applied to files:
lsc_agent_eval/src/lsc_agent_eval/core/utils/judge.py
🧬 Code graph analysis (1)
lsc_agent_eval/src/lsc_agent_eval/core/utils/judge.py (1)
lsc_agent_eval/src/lsc_agent_eval/core/utils/exceptions.py (1)
JudgeModelError(20-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests (3.13)
🔇 Additional comments (9)
src/lightspeed_evaluation/core/system/env_validator.py (1)
27-27: LGTM! Azure environment variable validation updated correctly.The required Azure environment variables have been updated to use the simplified naming convention (
AZURE_API_KEY,AZURE_API_BASE). Note thatAZURE_DEPLOYMENT_NAMEis intentionally not validated here since it has a fallback toconfig.modelin the manager (line 63 of manager.py).src/lightspeed_evaluation/core/llm/manager.py (1)
63-63: LGTM! Azure deployment name handling updated correctly.The code now reads from
AZURE_DEPLOYMENT_NAMEinstead ofAZURE_OPENAI_DEPLOYMENT_NAME, maintaining the same fallback logic toself.config.modelwhen the environment variable is not set.lsc_agent_eval/tests/core/utils/test_judge.py (1)
38-40: LGTM! Azure environment variables updated correctly in test.All three Azure environment variable names have been updated to use the simplified naming convention:
AZURE_API_KEY(wasAZURE_OPENAI_API_KEY)AZURE_API_BASE(wasAZURE_OPENAI_ENDPOINT)AZURE_DEPLOYMENT_NAME(wasAZURE_OPENAI_DEPLOYMENT_NAME)The test correctly validates that the JudgeModelManager constructs the model name as
azure/gpt-4-deployment.tests/unit/core/llm/test_llm_manager.py (1)
56-56: Test correctly updated for new Azure environment variable.The test now uses
AZURE_DEPLOYMENT_NAMEinstead ofAZURE_OPENAI_DEPLOYMENT_NAME, which aligns with the changes in the manager code atsrc/lightspeed_evaluation/core/llm/manager.py:63. All old Azure environment variable references (AZURE_OPENAI_API_KEY,AZURE_OPENAI_ENDPOINT,AZURE_OPENAI_DEPLOYMENT_NAME) have been completely removed from the codebase. The test correctly uses themockerfixture for patching.lsc_agent_eval/src/lsc_agent_eval/core/utils/judge.py (2)
54-54: LGTM!Error message correctly updated to reflect the new environment variable names.
48-50: The variable renaming is correct and fully compatible with LiteLLM. LiteLLM explicitly supportsAZURE_API_KEYandAZURE_API_BASEfor Azure OpenAI configuration, and these simplified names are consistently used throughout the lightspeed-evaluation codebase (env_validator, llm manager, and tests). This is an intentional migration to LiteLLM-standard naming conventions.README.md (1)
344-344: LGTM!Helpful clarification that the
modelfield should contain the Azure deployment name rather than the model name. This addresses a common source of confusion with Azure OpenAI configurations.tests/unit/core/system/test_env_validator.py (2)
41-42: LGTM!Test correctly updated to use the new Azure environment variable names. The test structure and coverage remain intact.
172-173: LGTM!Provider dispatcher test correctly updated with the new Azure environment variable names, ensuring complete test coverage of the validation flow.
2 participants
Description
fix azure env variable names for judgeLLM
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.