-
Notifications
You must be signed in to change notification settings - Fork 628
Fix read_console default to include 'log' messages #515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the read_console tool’s default message types to include standard log messages in addition to errors and warnings, without changing any other behavior or parameters. Sequence diagram for read_console default types including logsequenceDiagram
actor Developer
participant Server as ReadConsoleService
participant Unity as UnityInstance
Developer->>Server: read_console(ctx, action=None, types=None, format=None)
Server->>Server: action = 'get' (default)
Server->>Server: types = ['error','warning','log'] (default)
Server->>Server: format = 'plain' (default)
Server->>Unity: get_console_entries(types=['error','warning','log'])
Unity-->>Server: console_entries
Server-->>Developer: console_entries (errors, warnings, logs)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe read_console function's default Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Consider whether changing the default to include 'log' messages might surprise existing callers that rely on the previous error/warning-only behavior, and if so, gate this behind an explicit flag or versioned behavior.
- Including 'log' by default may significantly increase the volume of console entries returned; it may be worth confirming that upstream consumers and any pagination/limits are tuned for this higher throughput.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider whether changing the default to include 'log' messages might surprise existing callers that rely on the previous error/warning-only behavior, and if so, gate this behind an explicit flag or versioned behavior.
- Including 'log' by default may significantly increase the volume of console entries returned; it may be worth confirming that upstream consumers and any pagination/limits are tuned for this higher throughput.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Motivation
read_consoletool previously defaulted to only['error', 'warning'], which omitted ordinaryDebug.Logmessages.Description
typesinServer/src/services/tools/read_console.pyfrom['error', 'warning']to['error', 'warning', 'log']soDebug.Logoutput is returned without explicit filters.read_consolewas modified and explicittypesparameters remain respected.Testing
read_console(for exampleServer/tests/integration/test_read_console_truncate.py) were not run as part of this rollout.Codex Task
Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.