Add config option to suppress CE starvation warnings#39
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a configuration flag to control whether Cats Effect CPU starvation warnings are emitted, defaulting to suppression for end users while enabling warnings when developing Cellar itself.
Changes:
- Add
suppress-starvation-warningstoConfig(defaulttrue) and document it inREADME.md - Override
CellarApp.reportCpuStarvationto respect the new suppression setting - Check in
.cellar/cellar.conffor repo development and narrow.gitignoreto ignore only.cellar/cache
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new suppress-starvation-warnings config option and its default. |
| lib/src/cellar/Config.scala | Adds suppressStarvationWarnings to the typed config model with a default value. |
| cli/src/cellar/cli/CellarApp.scala | Suppresses Cats Effect CPU starvation reporting based on config. |
| CLAUDE.md | Adds guidance to keep docs in sync and notes code conventions. |
| .gitignore | Stops ignoring all of .cellar, continues ignoring .cellar/cache. |
| .cellar/cellar.conf | Repo-local config enabling starvation warnings during Cellar development. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cli/src/cellar/cli/CellarApp.scala
Outdated
| override def reportCpuStarvation(exceeded: FiniteDuration): IO[Unit] = | ||
| Config.default.flatMap { config => | ||
| if config.suppressStarvationWarnings then IO.unit | ||
| else super.reportCpuStarvation(exceeded) | ||
| } |
There was a problem hiding this comment.
reportCpuStarvation reloads the full config from disk every time a starvation event is reported, and it always uses Config.default (user+project config) rather than the config file selected via --config. This can be expensive during a starvation incident and also means users who explicitly set suppress-starvation-warnings = false in a custom config passed with --config still won’t see warnings.
Consider loading the config once during app startup/command execution (using the same configOpt used by the command, when present), caching just the boolean flag, and making reportCpuStarvation a pure check against that cached value (also defaulting safely if config loading fails).
- Add StarvationChecksConfig with `enabled` flag (default: false via reference.conf) - Override runtimeConfig to disable starvation checker when not enabled - Make Config fully synchronous (loadSync/bootstrap) so runtimeConfig can read it - Simplify configOpt from Opts[IO[Config]] to Opts[Config] - Add .cellar/cellar.conf with starvation-checks.enabled=true for dev use - Update .gitignore to track .cellar/ (except cache) - Document new config in README and add docs note to CLAUDE.md - Env var override: CELLAR_STARVATION_CHECKS_ENABLED Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rochala
force-pushed
the
suppress-starvation-warnings
branch
from
01f5060 to
4072565
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cli/src/cellar/cli/CellarApp.scala
Outdated
| private val configOpt: Opts[Config] = | ||
| Opts.option[Path]("config", "Path to config file", "c").orNone | ||
| .map(p => if p.isDefined then Config.loadSync(p) else Config.bootstrap) |
There was a problem hiding this comment.
Config.loadSync/Config.bootstrap can throw ConfigReaderException/IO exceptions; calling it inside Opts.map means failures happen during option parsing and may crash with a stack trace instead of a user-friendly CLI error. Consider using decline validation (mapValidated/orElse) to convert failures into a Help/ValidationFailed, or keep this as IO[Config] and handle errors when running the command.
| case class StarvationChecksConfig(enabled: Boolean) derives ConfigReader | ||
|
|
||
| object Config { | ||
| lazy val default: IO[Config] = load(None) | ||
| case class Config(mill: MillConfig, sbt: SbtConfig, starvationChecks: StarvationChecksConfig) derives ConfigReader |
There was a problem hiding this comment.
PR metadata describes a suppress-starvation-warnings option (default suppressed) and overriding reportCpuStarvation, but the implementation introduces starvation-checks.enabled and disables the checker via runtimeConfig. Please align the PR description (and any external references) with the actual config key name/semantics and the chosen implementation approach.
| starvation-checks { | ||
| # Enable Cats Effect CPU starvation warnings (default: false). | ||
| # Set to true during development or CI to surface warnings. | ||
| enabled = false # env: CELLAR_STARVATION_CHECKS_ENABLED | ||
| } |
There was a problem hiding this comment.
Because the runtime starvation-check behavior is determined from Config.bootstrap (loaded from default locations) during runtimeConfig, configs supplied via --config won’t affect whether warnings are suppressed/enabled. Either document this limitation, or provide a separate pre-runtime override mechanism (e.g., env/sysprop) that matches the README config guidance.
- Add StarvationChecksConfig with `enabled` flag (default: false via reference.conf) - Override runtimeConfig to disable CE starvation checker when not enabled - Replace IO-based Config.load with sync Config.global lazy val - Drop --config flag — config is loaded from default locations only - Check in .cellar/cellar.conf with starvation-checks.enabled=true for dev - Update .gitignore to track .cellar/ (except cache) - Document new config in README, add docs note to CLAUDE.md - Env var override: CELLAR_STARVATION_CHECKS_ENABLED Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Config is now accessed via Config.global throughout. Handlers default to it but accept an override for testing (e.g. custom mill binary). CellarApp no longer passes config explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
starvation-checks.enabledconfig option (default:false— disabled for end users)runtimeConfigto setcpuStarvationCheckInitialDelay = Duration.Infwhen disabledConfig.loadwith syncConfig.globallazy val--configflag — config is loaded from default locations only (user-level + project-level).cellar/cellar.confwithstarvation-checks.enabled = truefor developmentCELLAR_STARVATION_CHECKS_ENABLED=trueTest plan
./mill cli.compile— compiles./mill lib.test— all tests pass🤖 Generated with Claude Code