Skip to content

feat(ui): add debug-log visibility toggle for chat - #6

Merged
LiusCraft merged 1 commit into
mainfrom
feat/chat-debug-log-toggle
Mar 14, 2026
Merged

feat(ui): add debug-log visibility toggle for chat#6
LiusCraft merged 1 commit into
mainfrom
feat/chat-debug-log-toggle

Conversation

@LiusCraft

Copy link
Copy Markdown
Owner

Summary

  • add a new show_debug_logs setting (persisted in local app settings) and expose it in the Settings panel under 会话显示
  • classify System/Client/Tool/Trace entries as debug logs, and filter them from the chat stream when the toggle is off
  • default debug-log visibility to off for new users, while keeping the switch configurable at runtime

Validation

  • cargo test -p host-app-gpui

@opencode-agent

Copy link
Copy Markdown

Code Review Summary

Correctness ✓

  • The filtering logic correctly hides System/Client/Tool/Trace messages when show_debug_logs is false
  • State management follows the existing draft/apply pattern correctly
  • Persistence correctly defaults to false for new users

Code Quality Issues

  1. Inefficient double iteration (panel.rs:105-118):

    let filtered_messages = self.chat_messages.iter()...collect();
    let hidden_debug_message_count = self.chat_messages.len().saturating_sub(filtered_messages.len());

    The messages are iterated twice. Could be done in one pass with a fold or by counting during filtering.

  2. Missing tests: No test coverage for the debug log filtering logic or the toggle functionality.

Edge Cases

  • ✓ Empty message list handled correctly
  • None in persisted settings defaults to false
  • ⚠️ Consider: Does "follow latest" work correctly with filtered messages? (it does - total_messages uses filtered count)

Minor Suggestions

  • ChatRole::is_debug_log() could be const since it's a simple match
  • The Chinese text in settings panel could be more explicit about what "DEBUG 日志" means

Verdict

Approve - Functionally correct implementation. The double iteration is a minor optimization opportunity but not a regression risk. Missing tests should be addressed before merge.

New%20session%20-%202026-03-14T13%3A14%3A32.034Z
opencode session  |  github run

@LiusCraft
LiusCraft force-pushed the feat/chat-debug-log-toggle branch from 3c24a1c to 6a6f790 Compare March 14, 2026 15:54
@github-actions

Copy link
Copy Markdown

Based on my review, the PR is functionally correct. The implementation:

  1. Correctly filters System/Client/Tool/Trace messages when show_debug_logs is false
  2. Properly persists the setting with correct default (false for new users)
  3. Follows the draft/apply pattern for state management
  4. Renders correctly - the message_index passed to render functions refers to the original array, which is correct since the active message tracking operates on the original unfiltered array

The earlier review mentioned a double iteration inefficiency in panel.rs:103-112, but this is a minor optimization opportunity, not a correctness bug.

No issues found that meet the high-confidence, actionable threshold.

New%20session%20-%202026-03-14T15%3A55%3A10.967Z
opencode session  |  github run

@LiusCraft
LiusCraft merged commit 4ef9dae into main Mar 14, 2026
1 check 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.

1 participant