feat: move non-secret settings from .env to config.yaml#4
Conversation
Ensures containers recover automatically after crashes or Docker restarts, supporting Docker Desktop auto-start on Windows login. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
whisper.enabled/model/language/device, ollama.default_model, openai.model, and paths now live in config.yaml (volume-mounted, editable without rebuild). .env reduced to secrets only (OLLAMA_URL). Env var overrides still work. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Moves non-secret runtime settings from environment variables into a version-controlled config.yaml, keeping .env focused on secrets, and adjusts Docker Compose behavior for resilience.
Changes:
- Load non-secret settings (Ollama/OpenAI model, Whisper settings, paths, model filter) from
config.yamlwith limited env overrides. - Add
restart: unless-stoppedto theollamaandapiservices. - Expand
config.yamlwith defaults for Whisper/OpenAI/paths and a richer model filter list.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
voice-to-mermaid/backend/main.py |
Adds YAML config loading and switches several settings from env to config.yaml. |
voice-to-mermaid/backend/docker-compose.yml |
Adds restart policies for ollama and api. |
voice-to-mermaid/backend/config.yaml |
Adds non-secret settings previously sourced from env (Whisper/OpenAI/paths/default model). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Settings — config.yaml (env var override still works if needed) | ||
| OLLAMA_MODEL = os.environ.get("OLLAMA_MODEL") or _cfg.get("ollama", {}).get("default_model", "qwen3:8b") | ||
| OPENAI_MODEL = os.environ.get("OPENAI_MODEL") or _cfg.get("openai", {}).get("model", "gpt-4o-mini") | ||
| WHISPER_ENABLED = _cfg.get("whisper", {}).get("enabled", False) | ||
| WHISPER_MODEL = _cfg.get("whisper", {}).get("model", "medium.en") | ||
| WHISPER_LANG = _cfg.get("whisper", {}).get("language", "en") | ||
| WHISPER_DEVICE = _cfg.get("whisper", {}).get("device", "auto") | ||
| PROMPT_PATH = Path(_cfg.get("paths", {}).get("prompt", "prompts/mermaid.txt")) | ||
| LOG_DIR = Path(_cfg.get("paths", {}).get("log_dir", "data/logs")) |
There was a problem hiding this comment.
The comment says env var overrides still work for settings, but several previously supported env vars are no longer checked (e.g., WHISPER_ENABLED/WHISPER_MODEL/WHISPER_LANG/WHISPER_DEVICE, MERMAID_PROMPT_PATH, LOG_DIR). This is both a behavior change vs the previous env-only config and a discrepancy with the PR description; consider restoring env overrides (env takes priority) for these keys to preserve backwards compatibility.
| WHISPER_ENABLED = _cfg.get("whisper", {}).get("enabled", False) | ||
| WHISPER_MODEL = _cfg.get("whisper", {}).get("model", "medium.en") | ||
| WHISPER_LANG = _cfg.get("whisper", {}).get("language", "en") | ||
| WHISPER_DEVICE = _cfg.get("whisper", {}).get("device", "auto") |
There was a problem hiding this comment.
WHISPER_DEVICE is loaded from config.yaml but never used elsewhere in this module. Either wire it into the WhisperLiveKit configuration (so the setting actually takes effect) or remove it to avoid misleading configuration surface area.
| WHISPER_DEVICE = _cfg.get("whisper", {}).get("device", "auto") |
| model: "gpt-4o-mini" | ||
|
|
||
| whisper: | ||
| enabled: true |
There was a problem hiding this comment.
With config.yaml now version-controlled, whisper.enabled: true will make the API container attempt to import and initialize whisperlivekit on startup. The Docker image currently installs only requirements.txt (not requirements.stt.txt), so this will raise ModuleNotFoundError and crash by default. Consider defaulting whisper.enabled to false, adding STT deps to the image, or catching ImportError and auto-disabling STT with a clear log message.
| enabled: true | |
| enabled: false |
| def _load_config() -> dict: | ||
| try: | ||
| import yaml # type: ignore[import-untyped] | ||
|
|
||
| raw = yaml.safe_load(_CONFIG_PATH.read_text()) or {} | ||
| return [str(p) for p in (raw.get("ollama", {}).get("model_filter") or [])] | ||
| return yaml.safe_load(_CONFIG_PATH.read_text()) or {} | ||
| except FileNotFoundError: | ||
| return [] | ||
| log.warning("config.yaml not found — using defaults") | ||
| return {} | ||
| except Exception as exc: | ||
| log.warning("Failed to load config.yaml: %s", exc) | ||
| return [] | ||
| return {} |
There was a problem hiding this comment.
_load_config() returns the raw result of yaml.safe_load(), but later code assumes _cfg is a dict (calls _cfg.get(...)). If config.yaml is edited into a non-mapping YAML value (e.g., a list/string), the app will crash at import time with AttributeError. Consider validating the loaded value is a dict and otherwise falling back to {} with a warning.
…ck, WHISPER_DEVICE wired, default enabled=false
Summary
whisper.enabled,whisper.model,whisper.language,whisper.device— now inconfig.yamlollama.default_model,openai.model,paths.prompt,paths.log_dir— now inconfig.yaml.envreduced to secrets only (OLLAMA_URL,OPENAI_API_KEY)Why
Config settings like
WHISPER_ENABLEDshould live in a version-controlled, human-readable YAML file — not in.envwhich is for secrets. This means toggling whisper on/off is aconfig.yamledit, not a secret rotation.Test plan
docker compose up -d— confirmstt_enabled: truein/v1/configresponsewhisper.enabled: false→ restart → confirmstt_enabled: false🤖 Generated with Claude Code