Skip to content

RSPEED-2471: Use plain log handler in non-TTY environments#1201

Merged
tisnik merged 1 commit intolightspeed-core:mainfrom
major:fix/rich-handler-container-width
Feb 24, 2026
Merged

RSPEED-2471: Use plain log handler in non-TTY environments#1201
tisnik merged 1 commit intolightspeed-core:mainfrom
major:fix/rich-handler-container-width

Conversation

@major
Copy link
Contributor

@major major commented Feb 23, 2026

Description

RichHandler's columnar layout (timestamp, level, right-aligned filename) falls back to 80 columns when there's no TTY. The columns consume ~40 of those chars, leaving only ~35-40 for the actual log message. Tracebacks in containers become nearly unreadable.

Use sys.stderr.isatty() to pick the handler: RichHandler when a terminal is present (local dev), plain StreamHandler when not (containers, CI). No behavior change for local development.

Type of change

  • Bug fix

Tools used to create PR

  • Assisted-by: Claude

Related Tickets & Documents

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

Verified both code paths (TTY → RichHandler, no TTY → StreamHandler). All existing unit tests pass. All linters pass.

Summary by CodeRabbit

  • Improvements
    • Logging output now adapts to the execution environment: interactive terminals display logs with enhanced formatting for readability, while non-interactive environments receive plain, verbose text format for better compatibility
    • Invalid log level configurations handled gracefully with fallback to defaults and warning messages instead of failures

RichHandler's columnar layout falls back to 80 columns without a TTY,
leaving only ~40 chars for log messages. Tracebacks become unreadable.
Fall back to a plain StreamHandler when stderr is not a terminal.

Signed-off-by: Major Hayden <major@redhat.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2916319 and 68fa9e6.

📒 Files selected for processing (2)
  • src/lightspeed_stack.py
  • src/log.py

Walkthrough

Both files refactor logging initialization to conditionally adapt handler configuration based on TTY presence. Interactive terminals use RichHandler with simplified formatting, while non-interactive environments default to plain StreamHandler with verbose output including timestamps and line numbers.

Changes

Cohort / File(s) Summary
Logging Configuration
src/lightspeed_stack.py
Replaces unconditional logging.basicConfig with TTY-aware conditional setup. Uses RichHandler for interactive terminals with simple format; switches to verbose format with asctime, levelname, name, and lineno for non-TTY environments. Adds log level validation with fallback to default on invalid environment values.
Logger Initialization
src/log.py
Refactors logger setup to check for any existing handlers before reconfiguring, not just RichHandler presence. Implements environment-aware handler selection: RichHandler for TTY, plain StreamHandler for non-TTY. Preserves propagation settings and environment variable handling for log levels.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: switching to a plain log handler in non-TTY environments, which is the core problem being solved.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

ok, interesting bug + fix. TY

@tisnik tisnik merged commit 302bfb3 into lightspeed-core:main Feb 24, 2026
20 of 21 checks passed
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.

2 participants