ENG-10963 refactor(log): migrate the reflex CLI and runtime to logging (3/5) - #6865
Conversation
Greptile SummaryThe PR migrates Reflex CLI and runtime output to per-module Python loggers and introduces shared logging options, including JSON-lines output.
Confidence Score: 4/5The PR is not yet safe to merge because JSON/no-rich compilation still writes non-JSON progress lines to stdout. The prior reply states that the issue was fixed because Files Needing Attention: reflex/compiler/compiler.py and packages/reflex-base/src/reflex_base/utils/console.py
|
| Filename | Overview |
|---|---|
| reflex/compiler/compiler.py | Migrates compiler messages and timings to logging and changes progress construction; the previously reported JSON/no-rich progress defect remains outstanding. |
| reflex/utils/cli_options.py | Centralizes eager Click callbacks for log level, verbosity, and JSON mode. |
| reflex/reflex.py | Applies shared logging options across the main CLI and enables managed logging at the root command. |
| reflex/custom_components/custom_components.py | Migrates custom-component commands to module logging and shared CLI log options. |
| reflex/app.py | Replaces runtime console calls with a module logger across exception, page, and socket handling. |
Reviews (11): Last reviewed commit: "fix(istate): resolve the disk states dir..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
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".
There was a problem hiding this comment.
4 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/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="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
29505c0 to
b3d0c80
Compare
4ee5fbf to
754dbc5
Compare
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
2b2e21e to
3578b3d
Compare
3578b3d to
2942cd0
Compare
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).
- clear the deprecation dedupe set that console.deprecate actually uses in the legacy-token state manager test - scope the app tests' caplog assertions to the reflex.app logger - name the news fragment after the PR
…of the session Invoking the real ``cli`` group runs its callback, which sets REFLEX_MANAGED_LOGGING in os.environ and attaches the sinks for the whole process. Snapshot the marker with monkeypatch and detach the sinks after the invocation so later tests (and their subprocesses) start clean.
The default frontend/backend exception handlers now log through the pipeline. Under AppHarness the backend runs in-process in library mode, where records propagate to the root logger (pytest's caplog) and nothing is written to stderr, so the capsys assertions could never match. The React assertion also carried a line break from rich's console wrapping; log records are unwrapped.
StateManagerDisk resolved the relative .states path on every call, so a cwd change after startup (as AppHarness does) made every write-queue cycle raise FileNotFoundError from _purge_expired_states. Resolve it to an absolute path in the cached property. Log the expected closed-file ValueError in AppHarness at debug instead of error.
2942cd0 to
89a637c
Compare
client_error (#6827): mismatch scenarios produce actionable errors and fatal sessions as designed; sanitization/truncation/rate-limits verified via raw socket.io probing. Found: a no-argument client_error emit raises an unhandled TypeError, bypassing the anti-abuse guards. logging_cli (#6863/#6865/#6867/#6924): JSON-lines purity verified for reflex-owned logs across init/export/run; deprecation shims and missing hosting-cli fallbacks work. Found: REFLEX_ENABLE_FULL_LOGGING's worker file handler is closed (records silently dropped, stdout JSON purity broken) and granian's own lines break strict --json parsing at debug level. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
A ~29s 1920x1080 motion graphic covering the end-user-facing changes in 0.9.9, sourced from the 0.9.9a1 and 0.9.9a2 changelog entries across reflex, reflex-base, the component packages and reflex-hosting-cli. Nine beats: the dev-mode main-thread win (#6905), call-site auto-memoization of @rx.memo (#6949), the move to React Router 8 (#6854), DevTools component naming (#6945), stdlib logging with --json plus the client_error report (#6863, #6865, #6827), the cloud CLI's whoami/token and autoscaling flags (#6918, #6884, #6948), and a grid of the smaller fixes a user would notice (#6786, #6593, #6944, #6971, #6790, #6920). Styled to match reflex.dev by reusing its design system rather than approximating it: the dark violet/slate scales, wave-line motif, radial bloom and type scale come from reflex-site-shared, and the fonts are the same @fontsource-variable Instrument Sans and JetBrains Mono files the site serves. Source only — the rendered mp4 is a build artifact, regenerated by `npm run build` and ignored. Lives outside the packaged tree (hatch only includes /reflex), so it does not affect the published wheel or sdist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016arkVKmoogzUtZNfZZSvSo
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.