ENG-10963 refactor(log): migrate the reflex CLI and runtime to logging (3/5) - #6865
Conversation
Replace console.debug/info/warn/error call sites with per-module loggers across reflex/, add shared click log options in reflex.utils.cli_options including a new --json flag that switches output to JSON-lines records (REFLEX_LOG_JSON equivalent).
Greptile SummaryThe PR migrates Reflex CLI and runtime diagnostics from console helpers to per-module Python loggers and introduces shared
Confidence Score: 4/5The PR should not merge until JSON-mode compilation stops emitting plain-text progress records when --no-rich is selected. The new JSON output contract is broken by a directly reachable compile option combination because PoorProgress bypasses the JSON logging sink. Files Needing Attention: reflex/compiler/compiler.py, reflex/utils/cli_options.py
|
| Filename | Overview |
|---|---|
| reflex/utils/cli_options.py | Adds shared eager Click callbacks for log-level selection and JSON logging mode; callback composition preserves the prior command option surfaces. |
| reflex/reflex.py | Migrates primary CLI diagnostics and success messages to module logging and applies shared logging options to public commands. |
| reflex/compiler/compiler.py | Migrates compiler timing and diagnostics, but JSON mode combined with --no-rich still selects a progress implementation that emits plain text. |
| reflex/custom_components/custom_components.py | Migrates component CLI diagnostics and adopts shared log options while retaining direct console helpers for interactive presentation. |
| reflex/app.py | Replaces runtime application and websocket diagnostics with a module logger without changing control flow. |
| reflex/istate/manager/redis.py | Migrates Redis state-manager diagnostics while preserving deduplication metadata and lock-handling behavior. |
| reflex/model.py | Migrates database and migration diagnostics to standard logging without altering database operations. |
Reviews (1): Last reviewed commit: "refactor(log): migrate the reflex CLI an..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if use_rich | ||
| else console.PoorProgress() | ||
| ) | ||
| progress = console.progress() if use_rich else console.PoorProgress() |
There was a problem hiding this comment.
When reflex compile --json --no-rich performs a frontend compilation, this branch selects PoorProgress, whose advance method writes plain-text Progress: N/M lines directly to the console, causing JSON-lines consumers to fail parsing the command output.
Knowledge Base Used:
Merging this PR will not alter performance
Comparing 
Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29505c0626
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| @cli.command() | ||
| @loglevel_option | ||
| @log_options |
There was a problem hiding this comment.
When reflex login --json is used, this option enables the reflex_base JSON handler but then delegates to reflex_cli.v2.cli.login(), which prints messages such as “You already logged in” and “Successfully logged in” through reflex_cli.utils.console; that console always calls Rich's _console.print and never checks REFLEX_LOG_JSON. The resulting stdout mixes plain text with JSON records, so JSON-lines consumers cannot parse it. The same boundary affects other hosting-backed commands, so either bridge/migrate the hosting console before exposing --json on them or withhold this option there.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 39 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="reflex/reflex.py">
<violation number="1" location="reflex/reflex.py:96">
P2: The `login` command now exposes `--json` via `@log_options`, but it delegates to `reflex_cli.v2.cli.login()`, which prints through `reflex_cli.utils.console` (Rich's `_console.print`) and never checks the JSON log mode. Running `reflex login --json` will mix plain-text hosting CLI output with JSON-lines records, breaking JSON-lines consumers. Consider bridging/migrating the hosting console before exposing `--json` on hosting-backed commands, or withhold the option there until the hosting CLI is migrated.</violation>
<violation number="2" location="reflex/reflex.py:717">
P2: `reflex db` commands cannot accept the new `--json` flag, so their migrated log output is only JSON-capable through the environment variable. Apply `log_options` to the database subcommands (or provide equivalent group-level options) so CLI JSON output covers this command family.</violation>
</file>
<file name="tests/units/test_app.py">
<violation number="1" location="tests/units/test_app.py:352">
P3: These caplog assertions count records globally across every logger instead of scoping to the reflex.app logger/message, so any unrelated WARNING/ERROR emitted during the test (from add_page internals, config/deprecation paths, or pytest logging) turns the exact-count (`== 1`) assertion into a false failure. The previous targeted mocks (console.warn / console.error .assert_called_once) were immune to that noise; scope the filter to the app logger (e.g. r.name == "reflex.app") to keep the checks precise and stable.</violation>
</file>
<file name="tests/units/utils/test_utils.py">
<violation number="1" location="tests/units/utils/test_utils.py:790">
P2: This test no longer exercises the debug path it intends to. `output_system_info()` gates its body on `console.is_debug()` (`reflex_base.utils.console.is_debug`), which reads `console._LOG_LEVEL` — not `log._log_level`. The old test patched `reflex_base.utils.console._LOG_LEVEL` directly; the new one patches `reflex_base.utils.log._log_level`, which is a separate module global that only changes when `console.set_log_level()` is called. As a result `console.is_debug()` still returns `False` (default INFO), so `output_system_info()` returns early and the imports/iteration under the debug branch are no longer covered. The test still passes (its only assertion is that it doesn't crash), silently reducing coverage.</violation>
</file>
<file name="tests/units/test_state.py">
<violation number="1" location="tests/units/test_state.py:1927">
P3: The migrated `_clear_dedupe` clears `log._dedupe_filter().seen`, but `console.deprecate` still tracks its dedup in `_EMITTED_DEPRECATION_WARNINGS` (reflex_base/utils/console.py line 368) and never calls `dedupe_once`/`_dedupe_filter()`. So clearing the log filter's seen-set no longer resets the deprecation dedup the way the original `_base_console._EMITTED_DEPRECATION_WARNINGS -= ...` did; the helper is now a no-op against the storage `deprecate` actually uses. Point it back at `_EMITTED_DEPRECATION_WARNINGS` (or switch `deprecate` to the log dedupe and keep this).</violation>
</file>
<file name="reflex/compiler/compiler.py">
<violation number="1" location="reflex/compiler/compiler.py:1191">
P2: When `reflex compile` is run with `--json --no-rich`, `console.PoorProgress()` is selected as the fallback progress renderer, and its `advance` method writes plain-text `Progress: N/M` lines directly to stdout. This mixes non-JSON text into the output stream expected to be JSON-lines, breaking machine parsing for `--json` consumers.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # Check the database url. | ||
| if config.db_url is None: | ||
| console.error("db_url is not configured, cannot initialize.") | ||
| logger.error("db_url is not configured, cannot initialize.") |
There was a problem hiding this comment.
P2: reflex db commands cannot accept the new --json flag, so their migrated log output is only JSON-capable through the environment variable. Apply log_options to the database subcommands (or provide equivalent group-level options) so CLI JSON output covers this command family.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/reflex.py, line 717:
<comment>`reflex db` commands cannot accept the new `--json` flag, so their migrated log output is only JSON-capable through the environment variable. Apply `log_options` to the database subcommands (or provide equivalent group-level options) so CLI JSON output covers this command family.</comment>
<file context>
@@ -735,12 +714,12 @@ def db_init():
# Check the database url.
if config.db_url is None:
- console.error("db_url is not configured, cannot initialize.")
+ logger.error("db_url is not configured, cannot initialize.")
return
</file context>
| without crashing. | ||
| """ | ||
| mocker.patch("reflex_base.utils.console._LOG_LEVEL", constants.LogLevel.DEBUG) | ||
| mocker.patch("reflex_base.utils.log._log_level", constants.LogLevel.DEBUG) |
There was a problem hiding this comment.
P2: This test no longer exercises the debug path it intends to. output_system_info() gates its body on console.is_debug() (reflex_base.utils.console.is_debug), which reads console._LOG_LEVEL — not log._log_level. The old test patched reflex_base.utils.console._LOG_LEVEL directly; the new one patches reflex_base.utils.log._log_level, which is a separate module global that only changes when console.set_log_level() is called. As a result console.is_debug() still returns False (default INFO), so output_system_info() returns early and the imports/iteration under the debug branch are no longer covered. The test still passes (its only assertion is that it doesn't crash), silently reducing coverage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/utils/test_utils.py, line 790:
<comment>This test no longer exercises the debug path it intends to. `output_system_info()` gates its body on `console.is_debug()` (`reflex_base.utils.console.is_debug`), which reads `console._LOG_LEVEL` — not `log._log_level`. The old test patched `reflex_base.utils.console._LOG_LEVEL` directly; the new one patches `reflex_base.utils.log._log_level`, which is a separate module global that only changes when `console.set_log_level()` is called. As a result `console.is_debug()` still returns `False` (default INFO), so `output_system_info()` returns early and the imports/iteration under the debug branch are no longer covered. The test still passes (its only assertion is that it doesn't crash), silently reducing coverage.</comment>
<file context>
@@ -786,7 +787,7 @@ def test_output_system_info(mocker: MockerFixture):
without crashing.
"""
- mocker.patch("reflex_base.utils.console._LOG_LEVEL", constants.LogLevel.DEBUG)
+ mocker.patch("reflex_base.utils.log._log_level", constants.LogLevel.DEBUG)
utils_exec.output_system_info()
</file context>
| if use_rich | ||
| else console.PoorProgress() | ||
| ) | ||
| progress = console.progress() if use_rich else console.PoorProgress() |
There was a problem hiding this comment.
P2: When reflex compile is run with --json --no-rich, console.PoorProgress() is selected as the fallback progress renderer, and its advance method writes plain-text Progress: N/M lines directly to stdout. This mixes non-JSON text into the output stream expected to be JSON-lines, breaking machine parsing for --json consumers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/compiler/compiler.py, line 1191:
<comment>When `reflex compile` is run with `--json --no-rich`, `console.PoorProgress()` is selected as the fallback progress renderer, and its `advance` method writes plain-text `Progress: N/M` lines directly to stdout. This mixes non-JSON text into the output stream expected to be JSON-lines, breaking machine parsing for `--json` consumers.</comment>
<file context>
@@ -1176,24 +1179,16 @@ def compile_app(
- if use_rich
- else console.PoorProgress()
- )
+ progress = console.progress() if use_rich else console.PoorProgress()
fixed_steps = 7
compiler_plugins, radix_themes_plugin = _resolve_radix_themes_plugin(
</file context>
|
|
||
| @cli.command() | ||
| @loglevel_option | ||
| @log_options |
There was a problem hiding this comment.
P2: The login command now exposes --json via @log_options, but it delegates to reflex_cli.v2.cli.login(), which prints through reflex_cli.utils.console (Rich's _console.print) and never checks the JSON log mode. Running reflex login --json will mix plain-text hosting CLI output with JSON-lines records, breaking JSON-lines consumers. Consider bridging/migrating the hosting console before exposing --json on hosting-backed commands, or withhold the option there until the hosting CLI is migrated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/reflex.py, line 96:
<comment>The `login` command now exposes `--json` via `@log_options`, but it delegates to `reflex_cli.v2.cli.login()`, which prints through `reflex_cli.utils.console` (Rich's `_console.print`) and never checks the JSON log mode. Running `reflex login --json` will mix plain-text hosting CLI output with JSON-lines records, breaking JSON-lines consumers. Consider bridging/migrating the hosting console before exposing `--json` on hosting-backed commands, or withhold the option there until the hosting CLI is migrated.</comment>
<file context>
@@ -110,11 +86,14 @@ def _init(
@cli.command()
-@loglevel_option
+@log_options
@click.option(
"--name",
</file context>
| ) | ||
| app.add_page(second_page, route="/" + route.strip("/") if route else None) | ||
| assert mock_object.call_count == 1 | ||
| warnings = [r for r in caplog.records if r.levelno == logging.WARNING] |
There was a problem hiding this comment.
P3: These caplog assertions count records globally across every logger instead of scoping to the reflex.app logger/message, so any unrelated WARNING/ERROR emitted during the test (from add_page internals, config/deprecation paths, or pytest logging) turns the exact-count (== 1) assertion into a false failure. The previous targeted mocks (console.warn / console.error .assert_called_once) were immune to that noise; scope the filter to the app logger (e.g. r.name == "reflex.app") to keep the checks precise and stable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/test_app.py, line 352:
<comment>These caplog assertions count records globally across every logger instead of scoping to the reflex.app logger/message, so any unrelated WARNING/ERROR emitted during the test (from add_page internals, config/deprecation paths, or pytest logging) turns the exact-count (`== 1`) assertion into a false failure. The previous targeted mocks (console.warn / console.error .assert_called_once) were immune to that noise; scope the filter to the app logger (e.g. r.name == "reflex.app") to keep the checks precise and stable.</comment>
<file context>
@@ -344,17 +345,12 @@ def index():
- )
app.add_page(second_page, route="/" + route.strip("/") if route else None)
- assert mock_object.call_count == 1
+ warnings = [r for r in caplog.records if r.levelno == logging.WARNING]
+ assert len(warnings) == 1
</file context>
|
|
||
| def _clear_dedupe(): | ||
| _base_console._EMITTED_DEPRECATION_WARNINGS -= { | ||
| seen = log._dedupe_filter().seen |
There was a problem hiding this comment.
P3: The migrated _clear_dedupe clears log._dedupe_filter().seen, but console.deprecate still tracks its dedup in _EMITTED_DEPRECATION_WARNINGS (reflex_base/utils/console.py line 368) and never calls dedupe_once/_dedupe_filter(). So clearing the log filter's seen-set no longer resets the deprecation dedup the way the original _base_console._EMITTED_DEPRECATION_WARNINGS -= ... did; the helper is now a no-op against the storage deprecate actually uses. Point it back at _EMITTED_DEPRECATION_WARNINGS (or switch deprecate to the log dedupe and keep this).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/test_state.py, line 1927:
<comment>The migrated `_clear_dedupe` clears `log._dedupe_filter().seen`, but `console.deprecate` still tracks its dedup in `_EMITTED_DEPRECATION_WARNINGS` (reflex_base/utils/console.py line 368) and never calls `dedupe_once`/`_dedupe_filter()`. So clearing the log filter's seen-set no longer resets the deprecation dedup the way the original `_base_console._EMITTED_DEPRECATION_WARNINGS -= ...` did; the helper is now a no-op against the storage `deprecate` actually uses. Point it back at `_EMITTED_DEPRECATION_WARNINGS` (or switch `deprecate` to the log dedupe and keep this).</comment>
<file context>
@@ -1922,21 +1916,19 @@ async def test_state_manager_legacy_token(state_manager: StateManager, token: st
def _clear_dedupe():
- _base_console._EMITTED_DEPRECATION_WARNINGS -= {
+ seen = log._dedupe_filter().seen
+ seen -= {
k
</file context>
1 participant
Replaces the console helper call sites with per-module loggers across
reflex/, and adds shared click log options inreflex.utils.cli_optionsincluding a new--jsonflag that switches output to JSON-lines records (REFLEX_LOG_JSONequivalent).Keeps a temporary
LogLevelconversion mapper for the hosting CLI boundary; it is removed in the next PR when the enums unify.Stack (ENG-10963)
#6863 → #6864 → this → hosting CLI → deprecate.
Merge in order; each PR is based on the previous branch.