Add colored logs#14036
Conversation
Talmaj
requested review from
Kosinkadink,
alexisrolland,
comfyanonymous,
guill,
kijai and
rattus128
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds ANSI color formatting to log output. app/logger.py introduces a ColoredFormatter that prepends bold, color-coded [LEVEL] prefixes for WARNING and above and optionally colorizes message bodies when a record includes a color attribute. setup_logger is updated to use this formatter for stream and stdout handlers. main.py tags prompt execution timing logs with extra={'color': 'green'} so those messages render in green. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@Talmaj thank you for this PR, we discussed this with @comfyanonymous could you please remove the flag and make this the default behavior. Thanks! |
|
@Talmaj Thanks a lot for this feature. I always wanted to make a feature request about something like this but I feared is too frivolous. Since we are on this subject I think maybe here is the place to make a related feature request: |
|
@jovan2009 I am ok to do add this, but we'd need a confirmation from @comfyanonymous first. |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/logger.py (1)
72-72: ⚡ Quick winConsider terminal capability detection for robustness.
The colored logs are now enabled by default, which works well on modern terminals. However, ANSI escape codes will appear as literal text on terminals that don't support them (rare but possible) or when output is redirected to files.
For enhanced robustness, consider checking
sys.stdout.isatty()and/orsys.stderr.isatty()before defaulting to colored output. This is not critical since users can disable via--no-color-logs, but it would provide a better out-of-the-box experience in edge cases.🎨 Optional enhancement to detect terminal capability
-def setup_logger(log_level: str = 'INFO', capacity: int = 300, use_stdout: bool = False, color_logs: bool = True): +def setup_logger(log_level: str = 'INFO', capacity: int = 300, use_stdout: bool = False, color_logs: bool = True): global logs if logs: return @@ -86,7 +86,10 @@ logger = logging.getLogger() logger.setLevel(log_level) - formatter = ColoredFormatter("%(message)s") if color_logs else logging.Formatter("%(message)s") + # Auto-detect if terminal supports colors when color_logs is True + supports_color = color_logs and (sys.stderr.isatty() or (use_stdout and sys.stdout.isatty())) + + formatter = ColoredFormatter("%(message)s") if supports_color else logging.Formatter("%(message)s")Also applies to: 89-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/logger.py` at line 72, The setup_logger function currently enables color_logs by default; change its behavior to detect terminal capability instead: make color_logs optional (e.g., None) and inside setup_logger decide its effective value using sys.stdout.isatty() or sys.stderr.isatty() (choose stdout when use_stdout is True, otherwise stderr), so colors are only enabled when the chosen stream is a TTY; ensure explicit True/False passed by callers still overrides detection and keep use_stdout, color_logs and setup_logger as the referenced symbols to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/logger.py`:
- Line 72: The setup_logger function currently enables color_logs by default;
change its behavior to detect terminal capability instead: make color_logs
optional (e.g., None) and inside setup_logger decide its effective value using
sys.stdout.isatty() or sys.stderr.isatty() (choose stdout when use_stdout is
True, otherwise stderr), so colors are only enabled when the chosen stream is a
TTY; ensure explicit True/False passed by callers still overrides detection and
keep use_stdout, color_logs and setup_logger as the referenced symbols to locate
the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c4a84e7f-a0ab-4e7c-b9c1-520877778ec6
📒 Files selected for processing (2)
app/logger.pycomfy/cli_args.py
I discussed with @comfyanonymous and he said he approves this additional change
|
You guys made my day. I'm looking forward for this PR to be merged. Thanks a lot! |
…rompt executed in ... green
|
@alexisrolland @jovan2009 done. I've added coloring of text as a new feature. One can do: This will color only the "message" part, not the logging level part. |
|
Thanks @Talmaj could you also please remove the feature flag? |
|
@alexisrolland done |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main.py (1)
347-347:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVerify that green-only coloring meets the "bold and green" requirement.
The reviewer jovan2009 requested that the completion message be "bold and green", but the current implementation only applies green color via
extra={'color': 'green'}. TheColoredFormatterapplies bold only to level tags for WARNING and above (line 34 in app/logger.py), so these INFO-level messages will be green but not bold.If bold text is required for the message body (not just the
[INFO]tag), you'll need to either:
- Add a
boldattribute to theextradict and handle it inColoredFormatter.format(), or- Include ANSI bold codes directly in the formatted message string
Also applies to: 349-349
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main.py` at line 347, The INFO-level completion log uses extra={'color': 'green'} which only colors the tag; to make the message body "bold and green" update the logging call(s) (the logging.info at the prompt completion lines) to either include extra={'color': 'green', 'bold': True} and extend ColoredFormatter.format() to respect a 'bold' key when applying styles, or else embed ANSI bold sequences directly into the formatted message string; modify ColoredFormatter.format() to read the new 'bold' attribute (or the embedded codes will already render) so the message body is rendered bold and green.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@main.py`:
- Line 347: The INFO-level completion log uses extra={'color': 'green'} which
only colors the tag; to make the message body "bold and green" update the
logging call(s) (the logging.info at the prompt completion lines) to either
include extra={'color': 'green', 'bold': True} and extend
ColoredFormatter.format() to respect a 'bold' key when applying styles, or else
embed ANSI bold sequences directly into the formatted message string; modify
ColoredFormatter.format() to read the new 'bold' attribute (or the embedded
codes will already render) so the message body is rendered bold and green.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cbf6dd7-013e-4bec-b1a7-1f79f32018f3
📒 Files selected for processing (2)
app/logger.pymain.py
3 participants
Sometimes it's very hard to read the logs efficiently. This PR adds optional flag when starting comfyUI
python main.py --color-logsThis colors all the infos green, warnings orange and errors red. See the image:
I would personally set this as a default option.