Skip to content

ENG-10963 refactor(log): hosting CLI shares the reflex-base console and logging (4/5) - #6866

Open
FarhanAliRaza wants to merge 1 commit into
farhan/eng-10963-log-3-migrate-corefrom
farhan/eng-10963-log-4-hosting-cli
Open

ENG-10963 refactor(log): hosting CLI shares the reflex-base console and logging (4/5)#6866
FarhanAliRaza wants to merge 1 commit into
farhan/eng-10963-log-3-migrate-corefrom
farhan/eng-10963-log-4-hosting-cli

Conversation

@FarhanAliRaza

@FarhanAliRaza FarhanAliRaza commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

reflex-hosting-cli now depends on reflex-base: its forked console module and LogLevel enum become shims over the shared ones, and its logging goes through standard python logging.

  • The base console.ask gains the same str-typed overloads the forked console had (the flat str | None signature broke typing at the hosting call sites).
  • Intentional behavior deltas: debug output renders purple (was blue), errors go to stderr (was stdout), success messages are hidden at --loglevel warning.
  • The temporary LogLevel conversion mapper in reflex/reflex.py is removed.
  • The hosting-cli dev pin (reflex-base >= 0.9.8.post19.dev0) must be re-pinned at the 0.9.9 release.

Stack (ENG-10963)

#6863#6864#6865 → this → deprecate.
Merge in order; each PR is based on the previous branch.

Review in cubic

reflex-hosting-cli now depends on reflex-base: its forked console
module and LogLevel enum become shims over the shared ones, and its
logging goes through standard python logging. The base console.ask
gains the same str-typed overloads the forked console had. Debug
output renders purple (was blue), errors go to stderr (was stdout),
and success messages are hidden at --loglevel warning. The
now-redundant LogLevel conversion mapper in reflex/reflex.py is
removed.
@FarhanAliRaza
FarhanAliRaza requested a review from a team as a code owner August 10, 2026 20:36
@linear-code

linear-code Bot commented Aug 10, 2026

Copy link
Copy Markdown

ENG-10963

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR consolidates hosting-CLI console and log-level behavior onto reflex-base and migrates diagnostic output to the shared standard-logging pipeline.

  • Adds reflex-base as a hosting-CLI dependency and replaces the duplicated console and LogLevel implementations with compatibility shims.
  • Migrates hosting command diagnostics and success messages to module loggers while retaining direct console output for command data.
  • Removes the now-unnecessary log-level conversion in the main Reflex CLI and updates tests for the shared logging behavior.
  • Adds typed overloads to the shared console prompt helper.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code defect identified.

The shared LogLevel and console shims preserve hosting call compatibility, commands configure the shared logging pipeline before emitting migrated records, and the lockfile’s reported vulnerable dependency versions were not introduced or worsened by this change.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/utils/console.py Adds string-returning overloads for ask while retaining the existing runtime implementation.
packages/reflex-hosting-cli/src/reflex_cli/utils/console.py Replaces the forked console implementation with reflex-base re-exports and a legacy-compatible string log-level adapter.
packages/reflex-hosting-cli/src/reflex_cli/constants/base.py Re-exports the shared reflex-base LogLevel instead of maintaining a separate enum.
packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py Routes hosting diagnostics through module logging and emits successful deployment states through the shared SUCCESS level.
packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py Migrates application-command diagnostics and success output to the shared logging pipeline without changing command control flow.
packages/reflex-hosting-cli/src/reflex_cli/v2/cli.py Migrates login, logout, and deployment diagnostics to standard logging while preserving log-level initialization.
reflex/reflex.py Passes the now-shared LogLevel directly to hosting commands and removes the temporary enum conversion helper.
packages/reflex-hosting-cli/pyproject.toml Declares reflex-base as a runtime dependency and configures its local workspace source.
uv.lock Updates workspace dependency metadata for reflex-hosting-cli without introducing the reported vulnerable package versions.

Reviews (1): Last reviewed commit: "refactor(log): hosting CLI shares the re..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Aug 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing farhan/eng-10963-log-4-hosting-cli (963fc1b) with farhan/eng-10963-log-3-migrate-core (29505c0)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 27 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="tests/units/reflex_cli/v2/test_secrets.py">

<violation number="1" location="tests/units/reflex_cli/v2/test_secrets.py:89">
P1: These new caplog assertions will never capture the log records: the reflex logging pipeline sets propagate=False on the reflex_cli root logger in configure() (triggered by every command via console.set_log_level), so records on reflex_cli.v2.secrets don't reach the root logger where caplog attaches its handler. caplog.records will be empty and the tests fail. Capture at the reflex_cli logger instead (e.g. caplog.at_level / attach a handler to logging.getLogger('reflex_cli')) or assert on the rendered output.</violation>
</file>

<file name="packages/reflex-hosting-cli/pyproject.toml">

<violation number="1" location="packages/reflex-hosting-cli/pyproject.toml:21">
P2: The new `reflex-base >= 0.9.8.post19.dev0` dependency is an unpublishable `.dev` pin. The repo's publish gate (`uv run python scripts/check_min_deps.py --check-dev-pins`) fails for any package that declares a `*.dev` dependency pin, and this minimum will not be satisfiable from PyPI once reflex-hosting-cli is published (it only resolves locally through the added `[tool.uv.sources]` workspace entry). This is acknowledged as temporary in the PR description (re-pin at the 0.9.9 release), but it will block the hosting CLI publish gate in the meantime — worth tracking so it isn't merged/released with the dev pin still in place.</violation>
</file>

<file name="tests/units/reflex_cli/v2/test_cli.py">

<violation number="1" location="tests/units/reflex_cli/v2/test_cli.py:18">
P2: The caplog-based assertions in the migrated tests may not capture the records they expect. caplog listens on the root logger, so it only sees records that propagate up the logger hierarchy. But cli.logout/deploy call `console.set_log_level(...)` before emitting the messages under test, and that call configures the reflex-base pipeline (reflex_base/utils/log.py `configure()`), which sets `logger.propagate = False` on the `reflex_cli` root logger and attaches its own handler. After propagation is disabled, the `logger.error`/`logger.log(SUCCESS)` records are handled by the reflex_cli handler and never reach caplog, so these assertions can return empty. Worth verifying these tests actually pass in the full suite (order-dependently they may fail once any earlier test configures the pipeline), and if so, capture at the `reflex_cli` logger level or assert via the reflex-base handler instead of the root-level caplog.</violation>
</file>

<file name="tests/units/reflex_cli/v2/test_apps.py">

<violation number="1" location="tests/units/reflex_cli/v2/test_apps.py:365">
P2: These new assertions read records off the `caplog` fixture, which captures by attaching a handler to the root logger. But the shared reflex-base logging pipeline (on which this PR switches the hosting CLI) sets `logger.propagate = False` on the `reflex_cli` package-root logger (and `reflex_base`), so records emitted by `reflex_cli.v2.apps` never reach the root logger. If the pipeline's `bootstrap()`/`configure()` runs during the test (the commands call `console.set_log_level`, which routes into reflex_base logging), `caplog.records` will be empty and the strict equality asserts like `errors == ["get status failed: Invalid token"]` / `warnings == ["..."]` will fail (or, with `in`/`[-1]` checks, pass vacuously). Consider capturing from the `reflex_cli` logger directly (e.g. `caplog.set_level(..., logger="reflex_cli")`), or verify in the test env that propagation is not disabled, so the assertions actually observe the records.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

)

mock_console_error.assert_called_once_with("failed to retrieve secrets.")
errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: These new caplog assertions will never capture the log records: the reflex logging pipeline sets propagate=False on the reflex_cli root logger in configure() (triggered by every command via console.set_log_level), so records on reflex_cli.v2.secrets don't reach the root logger where caplog attaches its handler. caplog.records will be empty and the tests fail. Capture at the reflex_cli logger instead (e.g. caplog.at_level / attach a handler to logging.getLogger('reflex_cli')) or assert on the rendered output.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_cli/v2/test_secrets.py, line 89:

<comment>These new caplog assertions will never capture the log records: the reflex logging pipeline sets propagate=False on the reflex_cli root logger in configure() (triggered by every command via console.set_log_level), so records on reflex_cli.v2.secrets don't reach the root logger where caplog attaches its handler. caplog.records will be empty and the tests fail. Capture at the reflex_cli logger instead (e.g. caplog.at_level / attach a handler to logging.getLogger('reflex_cli')) or assert on the rendered output.</comment>

<file context>
@@ -79,7 +86,8 @@ def test_get_secrets_error(mocker: MockFixture):
     )
 
-    mock_console_error.assert_called_once_with("failed to retrieve secrets.")
+    errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR]
+    assert errors == ["failed to retrieve secrets."]
 
</file context>

"httpx >=0.25.1,<1.0",
"packaging >=24.2",
"platformdirs >=3.10.0,<5.0",
"reflex-base >= 0.9.8.post19.dev0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The new reflex-base >= 0.9.8.post19.dev0 dependency is an unpublishable .dev pin. The repo's publish gate (uv run python scripts/check_min_deps.py --check-dev-pins) fails for any package that declares a *.dev dependency pin, and this minimum will not be satisfiable from PyPI once reflex-hosting-cli is published (it only resolves locally through the added [tool.uv.sources] workspace entry). This is acknowledged as temporary in the PR description (re-pin at the 0.9.9 release), but it will block the hosting CLI publish gate in the meantime — worth tracking so it isn't merged/released with the dev pin still in place.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-hosting-cli/pyproject.toml, line 21:

<comment>The new `reflex-base >= 0.9.8.post19.dev0` dependency is an unpublishable `.dev` pin. The repo's publish gate (`uv run python scripts/check_min_deps.py --check-dev-pins`) fails for any package that declares a `*.dev` dependency pin, and this minimum will not be satisfiable from PyPI once reflex-hosting-cli is published (it only resolves locally through the added `[tool.uv.sources]` workspace entry). This is acknowledged as temporary in the PR description (re-pin at the 0.9.9 release), but it will block the hosting CLI publish gate in the meantime — worth tracking so it isn't merged/released with the dev pin still in place.</comment>

<file context>
@@ -18,9 +18,13 @@ dependencies = [
     "httpx >=0.25.1,<1.0",
     "packaging >=24.2",
     "platformdirs >=3.10.0,<5.0",
+    "reflex-base >= 0.9.8.post19.dev0",
     "rich >=13,<16",
 ]
</file context>

from reflex_cli.v2 import cli


def _log_messages(caplog: pytest.LogCaptureFixture, level: int) -> list[str]:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The caplog-based assertions in the migrated tests may not capture the records they expect. caplog listens on the root logger, so it only sees records that propagate up the logger hierarchy. But cli.logout/deploy call console.set_log_level(...) before emitting the messages under test, and that call configures the reflex-base pipeline (reflex_base/utils/log.py configure()), which sets logger.propagate = False on the reflex_cli root logger and attaches its own handler. After propagation is disabled, the logger.error/logger.log(SUCCESS) records are handled by the reflex_cli handler and never reach caplog, so these assertions can return empty. Worth verifying these tests actually pass in the full suite (order-dependently they may fail once any earlier test configures the pipeline), and if so, capture at the reflex_cli logger level or assert via the reflex-base handler instead of the root-level caplog.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_cli/v2/test_cli.py, line 18:

<comment>The caplog-based assertions in the migrated tests may not capture the records they expect. caplog listens on the root logger, so it only sees records that propagate up the logger hierarchy. But cli.logout/deploy call `console.set_log_level(...)` before emitting the messages under test, and that call configures the reflex-base pipeline (reflex_base/utils/log.py `configure()`), which sets `logger.propagate = False` on the `reflex_cli` root logger and attaches its own handler. After propagation is disabled, the `logger.error`/`logger.log(SUCCESS)` records are handled by the reflex_cli handler and never reach caplog, so these assertions can return empty. Worth verifying these tests actually pass in the full suite (order-dependently they may fail once any earlier test configures the pipeline), and if so, capture at the `reflex_cli` logger level or assert via the reflex-base handler instead of the root-level caplog.</comment>

<file context>
@@ -9,10 +10,24 @@
 from reflex_cli.v2 import cli
 
 
+def _log_messages(caplog: pytest.LogCaptureFixture, level: int) -> list[str]:
+    """Return the captured log messages emitted at the given level.
+
</file context>

assert result.exit_code == 0, result.output
mock_error.assert_called_once_with("get status failed: Invalid token")
errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR]
assert errors == ["get status failed: Invalid token"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: These new assertions read records off the caplog fixture, which captures by attaching a handler to the root logger. But the shared reflex-base logging pipeline (on which this PR switches the hosting CLI) sets logger.propagate = False on the reflex_cli package-root logger (and reflex_base), so records emitted by reflex_cli.v2.apps never reach the root logger. If the pipeline's bootstrap()/configure() runs during the test (the commands call console.set_log_level, which routes into reflex_base logging), caplog.records will be empty and the strict equality asserts like errors == ["get status failed: Invalid token"] / warnings == ["..."] will fail (or, with in/[-1] checks, pass vacuously). Consider capturing from the reflex_cli logger directly (e.g. caplog.set_level(..., logger="reflex_cli")), or verify in the test env that propagation is not disabled, so the assertions actually observe the records.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_cli/v2/test_apps.py, line 365:

<comment>These new assertions read records off the `caplog` fixture, which captures by attaching a handler to the root logger. But the shared reflex-base logging pipeline (on which this PR switches the hosting CLI) sets `logger.propagate = False` on the `reflex_cli` package-root logger (and `reflex_base`), so records emitted by `reflex_cli.v2.apps` never reach the root logger. If the pipeline's `bootstrap()`/`configure()` runs during the test (the commands call `console.set_log_level`, which routes into reflex_base logging), `caplog.records` will be empty and the strict equality asserts like `errors == ["get status failed: Invalid token"]` / `warnings == ["..."]` will fail (or, with `in`/`[-1]` checks, pass vacuously). Consider capturing from the `reflex_cli` logger directly (e.g. `caplog.set_level(..., logger="reflex_cli")`), or verify in the test env that propagation is not disabled, so the assertions actually observe the records.</comment>

<file context>
@@ -354,11 +361,17 @@ def test_deployment_status_http_error(mocker: MockFixture):
     assert result.exit_code == 0, result.output
-    mock_error.assert_called_once_with("get status failed: Invalid token")
+    errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR]
+    assert errors == ["get status failed: Invalid token"]
+
 
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant