feat: add centralized environment configuration and constants - #184
Conversation
Extracted from monster PR #171 as part of Phase 3A strategic decomposition. Environment configuration utilities: - Canonical is_truthy() helper for environment flag parsing - Supports common truthy values: '1', 'true', 'yes' (case-insensitive) - Whitespace-tolerant parsing with None handling Centralized constants: - Emotion model directory configuration with environment override - Default model path: /app/models/emotion-english-distilroberta-base - EMOTION_MODEL_DIR environment variable support Eliminates code duplication across modules and provides single source of truth. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a standardized environment flag parsing helper and centralizes emotion model directory constants with support for an EMOTION_MODEL_DIR override. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@uelkerd has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
✨ Finishing touches
🧪 Generate unit tests
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.
Pull Request Overview
This PR adds centralized environment configuration utilities and constants to eliminate code duplication. It creates standardized environment parsing and model configuration management for the SAMO project.
- Adds
is_truthy()helper function for consistent environment flag parsing - Centralizes emotion model directory configuration with environment override support
- Updates string quotes from single to double quotes for consistency
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/constants.py | Updates string quotes to double quotes for consistency in model directory constants |
| src/common/env.py | Removes trailing empty line for cleaner formatting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Summary of ChangesHello @uelkerd, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request centralizes environment configuration and application constants to enhance consistency and reduce code duplication. It introduces a standardized utility for parsing 'truthy' environment variables and establishes a single source of truth for the emotion model directory, which can be dynamically configured via an environment variable. This change is a strategic extraction from a larger refactoring effort, marking the completion of a significant decomposition phase. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Here's the code health analysis summary for commits Analysis Summary
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider applying .strip() to the EMOTION_MODEL_DIR value retrieved from the environment to avoid issues with accidental leading or trailing whitespace.
- It may be helpful to validate that the EMOTION_MODEL_DIR path exists or log a warning at startup to catch misconfigurations early.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider applying .strip() to the EMOTION_MODEL_DIR value retrieved from the environment to avoid issues with accidental leading or trailing whitespace.
- It may be helpful to validate that the EMOTION_MODEL_DIR path exists or log a warning at startup to catch misconfigurations early.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a good step towards centralizing configuration by introducing an is_truthy helper and consolidating model directory constants. The changes improve code maintainability by reducing duplication. I've added one comment to enhance the robustness of handling the EMOTION_MODEL_DIR environment variable to prevent potential issues with empty values.
| DEFAULT_EMOTION_MODEL_DIR = '/app/models/emotion-english-distilroberta-base' | ||
| EMOTION_MODEL_DIR = os.getenv('EMOTION_MODEL_DIR', DEFAULT_EMOTION_MODEL_DIR) | ||
| DEFAULT_EMOTION_MODEL_DIR = "/app/models/emotion-english-distilroberta-base" | ||
| EMOTION_MODEL_DIR = os.getenv("EMOTION_MODEL_DIR", DEFAULT_EMOTION_MODEL_DIR) |
There was a problem hiding this comment.
The current use of os.getenv with a default value does not account for cases where the EMOTION_MODEL_DIR environment variable is set to an empty string. This would result in EMOTION_MODEL_DIR being an empty string, potentially causing unexpected behavior like writing to the current working directory. Using an or fallback would more robustly handle both unset and empty environment variables.
| EMOTION_MODEL_DIR = os.getenv("EMOTION_MODEL_DIR", DEFAULT_EMOTION_MODEL_DIR) | |
| EMOTION_MODEL_DIR = os.getenv("EMOTION_MODEL_DIR") or DEFAULT_EMOTION_MODEL_DIR |
Summary
Strategic extraction from monster PR #171 as part of Phase 3A focused decomposition - FINAL EXTRACTION.
Adds centralized environment utilities and constants to eliminate code duplication:
is_truthy()helper with standardized behaviorEMOTION_MODEL_DIRenvironment variableKey Features
Environment Utilities (
src/common/env.py).strip()handlingCentralized Constants (
src/constants.py)EMOTION_MODEL_DIRenvironment variable support/app/models/...)Integration Points
scripts/ci/run_full_ci_pipeline.pyfor CI environment detectionTest Plan
is_truthy()edge cases (None, whitespace, case variations)🏆 Phase 3A Complete: All 5 high-impact extractions successfully completed!
🏰 Fortress-Protected Development: Final single-purpose micro-PR (2 files, configuration utilities)
🤖 Generated with Claude Code
Summary by Sourcery
Centralize environment configuration by introducing a standardized truthy parsing helper and consolidating emotion model path constants with support for an environment variable override.
New Features:
Enhancements: