Skip to content

fix: remove FilteredStream stdout/stderr wrapper#5326

Merged
greysonlalonde merged 1 commit into
mainfrom
fix/remove-filtered-stream
Apr 7, 2026
Merged

fix: remove FilteredStream stdout/stderr wrapper#5326
greysonlalonde merged 1 commit into
mainfrom
fix/remove-filtered-stream

Conversation

@greysonlalonde

@greysonlalonde greysonlalonde commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Removes FilteredStream class that wrapped sys.stdout and sys.stderr at import time to suppress LiteLLM noise
  • litellm.suppress_debug_info = True already handles this
  • The wrapper added a threading.Lock + string comparison to every print() call in the process and was not fork-safe for Celery prefork workers

Test plan

  • Verify no LiteLLM banner noise appears in output
  • Verify Rich console formatting still works correctly
  • Run existing test suite

Note

Low Risk
Low risk: removes an import-time sys.stdout/sys.stderr wrapper, mainly affecting console/log output; primary potential regression is LiteLLM banner noise reappearing in some environments.

Overview
Removes the FilteredStream proxy and the import-time wrapping of sys.stdout/sys.stderr that previously filtered LiteLLM-related banner output.

Cleans up related imports, relying solely on litellm.suppress_debug_info = True for noise suppression instead of intercepting all process output.

Reviewed by Cursor Bugbot for commit e68975e. Bugbot is set up for automated code reviews on this repo. Configure here.

Wrapping sys.stdout and sys.stderr at import time with a
threading.Lock is not fork-safe and adds overhead to every
print call. litellm.suppress_debug_info already silences the
noisy output this was designed to filter.
@github-actions github-actions Bot added the size/S label Apr 7, 2026

@iris-clawd iris-clawd 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.

Review: Remove FilteredStream

Excellent cleanup. This is the kind of change that silently fixes a whole class of bugs.

✅ Strong approve

  • The FilteredStream was wrapping sys.stdout/sys.stderr at import time — every print() in the entire process went through a threading.Lock + str.lower() + substring match. That's measurable overhead on hot paths.
  • Not fork-safe: Celery prefork workers would inherit the locked stream, risking deadlocks if a fork happened while the lock was held.
  • __getattr__ delegation on IO streams is a footgun — libraries like Rich, logging handlers, and anything checking stream.buffer or stream.encoding can behave unpredictably with proxied streams.
  • litellm.suppress_debug_info = True (already set at line 98) is the correct upstream solution.

No concerns. Ship it. 💬 175

@iris-clawd iris-clawd 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.

Approved. Ship it. ✅

@greysonlalonde greysonlalonde merged commit b23b269 into main Apr 7, 2026
50 checks passed
@greysonlalonde greysonlalonde deleted the fix/remove-filtered-stream branch April 7, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants