fix(cli): install log sinks at startup so internals stay off the terminal - #169
Merged
Conversation
…inal Reported in #167. loguru ships a stderr sink at DEBUG level and nothing on the CLI path ever replaced it — `setup_logging()` was defined, exported and documented, but never called. Confirmed by inspecting the handler table after importing the CLI: one sink, id 0, level 10. The visible symptom is the line `load_config` emits for each absent config layer, which is every directory without a project-level deepcode_config.json: DEBUG | core.config:_load_raw:580 - deepcode_config.json not found at /Users/.../deepcode_config.json; skipping layer The reporter also noted `LoggerConfig.level` had no consumer, so the config offered no way to change this. Both follow from the same missing call. Ordering is the awkward part, and the reason this is not one line: reading the config is itself something that logs. So a quiet default goes in first, and the configured level is applied afterwards only when it differs. DEEPCODE_LOG_LEVEL short-circuits both, needs no config file, and matches what `deepcode mcp` already honours — it is the escape hatch for debugging config loading itself. Placed in deepcode.main(), which is the sole console_scripts entry and dispatches all ten subcommands. The report suggested the TUI and loop entrypoints; putting it in each would be the same block copied ten times, and the eleventh subcommand would forget it. Verified from a directory with no project config: sink level moves 10 -> 20 and the DEBUG line stops, while DEEPCODE_LOG_LEVEL=DEBUG brings it back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #167.
Confirmed as reported
All three claims check out:
setup_logging()is never called. Every occurrence in the tree is a definition, a re-export or a docstring. Inspecting the handler table after importing the CLI shows loguru's own default still in place: one sink, id0, level10(DEBUG), aStreamSinkon stderr.core/config.py:_load_raw, which logs once per absent config layer — every directory without a project-leveldeepcode_config.json.LoggerConfig.levelhas no consumer, so the config offered no way to turn it down.All three follow from the same missing call.
The ordering problem
Reading the config is itself one of the things that logs, so
setup_logging(load_config().logger)would emit the very lines it is meant to suppress. A quiet default goes in first; the configured level is applied afterwards, and only when it differs from it.DEEPCODE_LOG_LEVELshort-circuits both. It needs no config file, which makes it the escape hatch for debugging config loading itself, and it matches whatdeepcode mcpalready honours.One call site, not two
The report suggested
cli/tui/app.pyandcli/loop_cli.py. This lands indeepcode.main()instead:setup.pydeclares a singleconsole_scriptsentry (deepcode=deepcode:main) and that function dispatches all ten subcommands. Per-entrypoint calls would be the same block copied ten times, and the eleventh subcommand would forget it.A broken config is caught and ignored here — logging is already usable at that point, and reporting a bad config properly is the subcommand's job.
Verification
From a directory with no project config:
load_config()outputDEEPCODE_LOG_LEVEL=DEBUGFive tests cover the sink level, the env override with no config present, a configured
warninglevel, a malformed config, and thatmain()bootstraps before dispatching. Removing the call frommain()fails the last one.Full suite: 1223 passed, 3 skipped.