Skip to content

Conversation

@DerekStride
Copy link
Member

@DerekStride DerekStride commented Jan 21, 2026

Summary

  • Introduces Roast::Log module as the central logging interface (addresses Roast::Log instead of Roast::Helpers::Logger #390)
  • Outputs to $stderr by default so the logs can be redirected separate from workflow output, it's also testable via Minitest's capture_io
  • Supports custom loggers via Roast::Log.logger = custom_logger
  • Respects ROAST_LOG_LEVEL env var for log level configuration
  • Updates claude_invocation.rb to use Roast::Log for unhandled message warnings

🤖 Generated with Claude Code

Introduces a clean, testable logging interface at Roast::Log that:
- Provides standard log methods: debug, info, warn, error, fatal
- Outputs to $stderr (testable via Minitest's capture_io)
- Respects ROAST_LOG_LEVEL env var for log level configuration
- Has reset! method for test isolation

This addresses issue #390 (rename Roast::Helpers::Logger to Roast::Log).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@mathiusj mathiusj left a comment

Choose a reason for hiding this comment

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

This lgtm. There are a couple other files that could benefit from using this logger, but not opposed to following up with those code changes 🙏 or if you're open to adding them to this PR, that would be great!

image.png

Copy link
Contributor

@nfgrep nfgrep left a comment

Choose a reason for hiding this comment

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

LGTM

@DerekStride
Copy link
Member Author

I added another commit (dca0250) to update some of the obvious places that would fall clearly under logs / stderr output in my opinion.

There are other places like the following that I thought might fall under output / stdout which I didn't update. I thought that having the config.show_prompt! / config.show_response! / config.show_stats! conflict with ROAST_LOG_LEVEL would result in a confusing user experience. However, I'm happy to update those if you think they should go through the Log class. Alternatively, we can expose a Log#show_prompt and similar methods that respect the config & possibly go through a logger with a different formatter. I think that can be addressed in a separate PR if there's interest though.

when :user
puts "[USER PROMPT] #{message.content}" if config.show_prompt?
when :assistant
puts "[LLM RESPONSE] #{message.content}" if config.show_response?

@dersam
Copy link
Contributor

dersam commented Jan 27, 2026

Let's keep the config options as they are - using the logger doesn't feel right. Logs are informational, while the config options are workflow specific. I don't think they should be beholden to log level.

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.

Roast::Log instead of Roast::Helpers::Logger

4 participants