refactor(output): route remaining raw rich.print imports through the JSON-mode-aware rprint shim

[mattmillerai]

ELI-5

comfy-cli speaks to two audiences: humans (pretty Rich-formatted text) and agents (a machine-readable JSON/NDJSON envelope on stdout). To keep those apart there's a small shim — comfy_cli.output.rprint — that behaves exactly like rich.print for humans but, in --json/agent mode, sends the pretty output to stderr so stdout stays a clean, parseable envelope.

Eight modules still imported the raw from rich import print. This PR migrates the five whose output is a log/hint (stderr in JSON mode is exactly right for those), and deliberately leaves the three result-producing modules alone — migrating those would move a command's primary result off stdout with nothing put back. See "Why three modules are intentionally not migrated" below.

What changed

Per the migration policy documented in comfy_cli/output/__init__.py:

Migrated (5) — log/hint output

File	Change
comfy_cli/resolve_python.py	from comfy_cli.output import rprint
comfy_cli/pr_cache.py	from comfy_cli.output import rprint
comfy_cli/uv.py	function-local → from comfy_cli.output import rprint
comfy_cli/command/custom_nodes/cm_cli_util.py	from comfy_cli.output import rprint as print (shadows builtin, matches models.py precedent)
comfy_cli/output/preview.py	function-local → from comfy_cli.output import rprint

Intentionally NOT migrated (3) — result-producing

File	Why
comfy_cli/command/generate/app.py	generate's primary result (job ids, image URLs); has its own --json
comfy_cli/command/generate/output.py	print_urls / print_saved are generate's primary result
comfy_cli/command/pr_command.py	pr list renders via a module-level Console; migrating only the rprint calls would split one command across two streams

Each carries an in-file comment explaining why, so the mechanical migration isn't blindly redone later.

Why three modules are intentionally not migrated

The shim routes to stderr whenever the renderer is in JSON mode, and Renderer.resolve() auto-selects JSON mode when stdout isn't a TTY (renderer.py, precedence rule 6). That's correct only for call sites whose text is a log — JSON mode reserves stdout for the envelope from renderer.emit().

generate and pr never emit an envelope on their main paths. Migrating them moved their primary result to stderr and put nothing back on stdout. Verified on the branch before the fix (CliRunner, non-TTY):

comfy generate flux-pro --prompt x --async
exit=0  stdout=''  stderr='Submitted: flux-pro\n  job id: job-xyz\n  resume: ...'

stdout was completely empty — comfy generate ... > out.txt would have written an empty file. 25 tests caught this; commit e365aa3 reverts those three modules. They should be migrated together with renderer.emit(...) envelope support for generate / pr, not before.

Why it's safe

• Pretty mode is byte-identical — the shim routes to a default pretty Renderer (stdout) and calls the real rich.print.
• get_renderer() has a safe default (a pretty Renderer when none is installed), so the two top-level modules (resolve_python.py, pr_cache.py) that import the shim at module top can't blow up on early/ad-hoc import.
• No circular import — verified by importing all eight modules in fresh interpreters.
• cm_cli_util.py calls print(..., file=sys.stderr) in several places; Renderer.print uses setdefault("file", ...), so an explicit file= is preserved and those calls are unchanged, while file-less calls now correctly route to stderr in JSON mode (the intended improvement).

Testing

• ruff check + ruff format --check clean on all touched files.
• Full suite: 2573 passed, 37 skipped.
• Shim routing verified empirically: JSON mode → stdout empty / text on stderr; pretty mode → stdout.
• generate's primary result confirmed back on stdout on a non-TTY.

CI note — the two red checks are NOT from this PR

• build — 9 failures in test_config_parser.py (8) + test_node_init.py (1): ValueError: Comment cannot contain line breaks. tomlkit is unpinned (pyproject.toml:49); CI resolves 0.15.1, which rejects line breaks in Item.comment(), and config_parser.py:75 passes a multi-line comment. Already being fixed by fix(registry): emit pyproject hint blocks as single-line tomlkit comments #533. This PR touches nothing near config_parser.py.
• test (Windows Specific Commands) — fails in Install Dependencies, before any of this code runs: failed to remove file ...pydantic_core/_pydantic_core.cp312-win_amd64.pyd: Access is denied (os error 5), leaving a corrupted venv → ImportError: cannot import name '__version__' from 'pydantic_core'. Reproduces on every recent branch (4/4 checked) and survived a re-run — a repo-wide Windows workflow break, not this PR.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 459d4d11-8fcb-4540-bb79-dc37f381feca

📥 Commits

Reviewing files that changed from the base of the PR and between c9b34b8 and 73bd066.

📒 Files selected for processing (4)

• comfy_cli/command/generate/app.py
• comfy_cli/command/generate/output.py
• comfy_cli/command/pr_command.py
• comfy_cli/output/__init__.py

📝 Walkthrough

Walkthrough

The changes replace direct rich.print imports with comfy_cli.output.rprint across CLI commands, generation output, cache handling, video preview, Python resolution, and UV error reporting. Existing Rich component imports and runtime logic remain unchanged.

Changes

Context-aware output import migration

Layer / File(s)	Summary
Replace direct Rich printing imports comfy_cli/command/custom_nodes/cm_cli_util.py, comfy_cli/command/generate/..., comfy_cli/command/pr_command.py, comfy_cli/output/preview.py, comfy_cli/pr_cache.py, comfy_cli/resolve_python.py, comfy_cli/uv.py	Existing output calls now use comfy_cli.output.rprint; required Rich components remain imported directly. A tidy little print-switch, no logic twist.

Suggested reviewers: bigcat88, robinjhuang

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-3270-rprint-shim

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-3270-rprint-shim

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

✅ No high-signal findings.

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)

[mattmillerai]

CI triage + scope correction

No unresolved review threads (CodeRabbit ✅, Cursor panel: no high-signal findings). CI was red for two unrelated reasons; only one was this PR's.

1. 25 failing generate tests — this PR's, fixed in e365aa3

The rprint shim sends human text to stderr whenever the renderer is in JSON mode, and Renderer.resolve() auto-selects JSON mode when stdout isn't a TTY (renderer.py, precedence rule 6). That's only correct for call sites whose text is a log — JSON mode reserves stdout for the envelope from renderer.emit().

generate and pr never emit an envelope on their main paths: generate has its own local --json (_emit_result → output.print_json) and pr list renders through a plain module-level Console. Migrating them moved their primary result off stdout and put nothing back. Verified on the branch before the fix (CliRunner, non-TTY):

comfy generate flux-pro --prompt x --async
exit=0  stdout=''  stderr='Submitted: flux-pro\n  job id: job-xyz\n  resume: ...'

stdout was completely empty — so comfy generate ... > out.txt would have written an empty file. That's what the 25 tests were catching.

So I reverted the three result-producing modules (generate/app.py, generate/output.py, pr_command.py) to from rich import print, each with a comment explaining why, so the mechanical migration isn't just redone later. The five log/hint modules stay migrated (cm_cli_util, preview, pr_cache, resolve_python, uv) — stderr in JSON mode is exactly right for those. This matches the module's own stated policy, which pairs migration with renderer.emit(data) for commands that produce a structured result.

Local after fix: 2564 passed, all 172 generate tests green, ruff check/format clean.

2. 9 remaining failures — pre-existing on main, not this PR

test_config_parser.py (8) + test_node_init.py (1) fail with ValueError: Comment cannot contain line breaks. tomlkit is unpinned (pyproject.toml:49), CI resolves 0.15.1, which now rejects line breaks in Item.comment() — and config_parser.py:75 passes a multi-line comment. Reproduced on origin/main's source, so this is independent of #523. Deferred to a follow-up rather than folded into an import refactor.

The Windows test job is also unrelated — uv pip install --requirement requirements.compiled exits 2 and leaves a broken venv (pydantic_core unimportable); the uv.py frame in that traceback is just the hint path printing after the install already failed.

Follow-ups proposed

• [high] Fix/pin tomlkit — create_comfynode_config passes a multi-line comment (unblocks 9 tests repo-wide).
• [medium] Emit renderer envelopes from generate/pr, then migrate their rich.print. Needs an owner's call on how generate's existing --json (raw partner-API body) reconciles with the envelope contract — that's a breaking change to an existing machine-readable output, so I didn't blind-ship it.

[mattmillerai]

CI result after e365aa3

build: 45 failed → 9 failed (2565 passed). All 25 generate failures are gone; ruff_check ✅.

The 9 that remain are exactly the pre-existing tomlkit breakage described above — test_config_parser.py (8) + test_node_init.py (1), all ValueError: Comment cannot contain line breaks — reproduced identically against origin/main's source, so they are not from this PR and will stay red on any PR in the repo until tomlkit is fixed/pinned. Tracked as the high-severity follow-up rather than smuggled into an import refactor.

⚠️ So this PR is functionally ready but cannot go green until that separate tomlkit fix lands. Not merging — flagging for a maintainer's call on ordering.

[mattmillerai]

Updated the PR description to match the final state of the branch — it still described all 8 modules as migrated, which stopped being true in e365aa3.

Final state: 5 log/hint modules migrated to the shim, 3 result-producing modules intentionally left on raw rich.print (generate/app.py, generate/output.py, pr_command.py), each with an in-file comment explaining why. Migrating those would move a command's primary result to stderr on any non-TTY stdout, leaving comfy generate ... > out.txt empty; they should move together with renderer.emit(...) envelope support, not before.

Re-verified this pass: full suite 2573 passed, 37 skipped; ruff check/format clean; all 8 modules import cleanly in fresh interpreters (no cycle); shim routing confirmed empirically (JSON → stderr with stdout clean, pretty → stdout); and generate's result confirmed back on stdout on a non-TTY.

Both red checks are pre-existing and not from this PR — build is the unpinned-tomlkit 0.15.1 breakage already being fixed by #533, and the Windows test job dies in Install Dependencies on a locked pydantic_core .pyd (fails on 4/4 other recent branches and survived a re-run). Details in the description.

@coderabbitai review

[coderabbitai]

@mattmillerai Thanks for the scope correction and thorough CI triage. I’ll review the final diff with the output-contract distinction in mind: migrate log/hint paths to the JSON-aware shim, while retaining raw rich.print for primary generate/pr results until envelope support is introduced. Output without an envelope is a hare-y situation. 🐇

---

✏️ Learnings added

Learnt from: mattmillerai
URL: https://github.com/Comfy-Org/comfy-cli/pull/523

Timestamp: 2026-07-17T06:41:57.423Z
Learning: In comfy-cli, `comfy_cli.output.rprint` routes non-envelope human output to stderr when the renderer resolves JSON mode (including non-TTY stdout). Log/hint-only modules should use this shim, but primary result-producing paths must retain `rich.print` until they emit a `renderer.emit(...)` envelope. As of PR `#523`, the intentionally raw result paths are `comfy_cli/command/generate/app.py`, `comfy_cli/command/generate/output.py`, and `comfy_cli/command/pr_command.py`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mattmillerai]

Independent re-verification + one docstring fix (73bd066)

Re-checked this branch from a clean worktree rather than taking the earlier triage on faith.

Both red checks reproduce on main's source — neither is from this PR.

• build — installed the same tomlkit 0.15.1 CI resolves, then ran the failing files twice in one venv: 9 failed on this branch, the same 9 failed on origin/main's source (test_config_parser.py ×8 + test_node_init.py ×1, all ValueError: Comment cannot contain line breaks). Fixed by fix(registry): emit pyproject hint blocks as single-line tomlkit comments #533, which is still open — this PR can't go green until that lands.
• test (Windows) — dies in Install Dependencies on a locked pydantic_core .pyd (os error 5), before any of this code runs.

Full suite here: 2564 passed, 37 skipped, 9 failed (exactly the tomlkit set). ruff check + ruff format --check clean.

The non-migration decision holds up empirically. Renderer.resolve() precedence rule 6 selects JSON whenever stdout isn't a TTY, and I confirmed the shim's routing in subprocesses: JSON mode → stdout='', text on stderr; --no-json pretty → stdout='LOG-LINE\n'. So migrating a result path really would leave comfy generate ... > out.txt empty. Leaving the three modules on raw rich.print is correct.

One thing I did change. The Phase 1 policy in comfy_cli/output/__init__.py still read "Every existing from rich import print as rprint becomes from comfy_cli.output import rprint" — the blanket instruction that motivated migrating generate/pr in the first place, and would motivate redoing it later. The new in-file comments warned against a policy that still said to do it. Scoped the rule to log/hint output, stated the result-path carve-out and why, and named the three un-migrated modules so those comments have something to point back to. Docstring only — no behavior change.

[mattmillerai]

Independent re-verification pass (clean worktree). No code changes — nothing actionable found.

• No merge conflict — MERGEABLE against main.
• No unresolved review threads — CodeRabbit ✅, Cursor panel clean.
• ruff check + ruff format --check clean.

Both red checks re-confirmed as not this PR's, by direct comparison rather than inspection:

• build — ran the failing files against origin/main's source with the exact tomlkit CI resolves (0.15.1): the same 9 fail there (test_config_parser.py ×8 + test_node_init.py ×1, all ValueError: Comment cannot contain line breaks, from the multi-line .comment() at config_parser.py:75). Full suite on this branch: 2564 passed, 37 skipped, 9 failed — byte-identical failure set to main. This PR touches nothing under registry/ or nodes/.
• test (Windows) — dies in Install Dependencies on a locked pydantic_core .pyd, before any of this code runs.

New this pass — the unblock path is verified, not assumed: I checked out #533 and ran those same files in the same tomlkit-0.15.1 venv: 87 passed, 0 failed. So #533 genuinely clears all 9, and #523 goes green the moment #533 merges. Suggested ordering: land #533 first, then this.

Also re-checked the riskiest edit adversarially — cm_cli_util.py shadows the builtin print module-wide. All 6 call sites pass only rich.print-compatible kwargs (file= on two), and Renderer.print uses kwargs.setdefault("file", ...), so the explicit file=sys.stderr calls keep their stream. No behavior change in pretty mode.

Follow-ups already tracked: the tomlkit fix is #533; the generate/pr envelope migration was filed previously. Not merging — leaving the ordering call to a maintainer.