Repository navigation
Add a live Logs window and a Verbose logging switch - #154
Conversation
… Verbose logging switch that takes effect in all three without a restart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
unRekable
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved ✅ (0 critical, 0 blocking, 4 suggestions) — read the full diff plus surrounding code.
💡 Suggestions
- log_tail.rs:155 — Rotation can slip through when the fresh live file grows past the old offset within one 500 ms poll (needs roughly sustained multi-MiB/s output), and the read then silently starts mid-file. A version/id check (creation time or first-line fingerprint) would make roll detection exact.
- gui/logs.rs:217 — Filter edits while paused recompute
visiblefromself.tail, which already holds the lines that arrived during the pause, so pauses are not fully still. Clampingvisibleto the paused seq would make the semantics uniform. - log_policy.rs:313 —
TINYCLIENT_LOG_LEVELonshare_sessionchildren is currently untested; oneget_envsassertion keyed on a guard being installed would cover it. - log_tail.rs:241 —
contains_foldedfolds ASCII only, so the filter doc slightly overstates the case-insensitivity for non-ASCII terms. Either say ASCII, or fold viachar::to_lowercase(keep the ASCII fast path).
✅ Looks Good
- The tail engine handles the hard cases for real: backfill of at most 1 MiB from the end dropping the mid-line it lands in, rotation (writer reopens the fresh live file,
lenshrinks), 64 KiB-capped partials, UTF-8 multibyte splits across reads, 5000-line bound per source — each with a unit test. - The redaction guarantee holds end to end: the window only reads files whose lines already went through
log_policy::render(redact + cut + denylist), and no user-controlled path is involved. - Verbose logging reuses the existing
relevelpattern consistently; unknown words leave mediad's ceiling alone, both probe and serve answerDone, and theLogLevelwire frame round-trips (old frames still parse). - Polling claims check out: reads run on the background executor, the loop breaks when the window entity is gone, per-poll bytes are bounded, memory is ring-bounded and tested.
CI note: both failing test jobs are the chatd --test playback timing flake (paused at 602 vs a ≤600 bound right after a 400 ms sleep). I re-ran a_pause_lets_the_process_go_and_play_goes_on_from_there from a checkout of this branch — it passes on Windows. The diff touches no playback code. Please re-run the failed jobs so the branch goes green.
| let len = file.metadata()?.len(); | ||
| let (from, mid_line) = match self.offset { | ||
| None => (len.saturating_sub(BACKFILL), len > BACKFILL), | ||
| Some(offset) if offset > len => { |
There was a problem hiding this comment.
💡 Suggestion: Roll detection via offset > len alone can miss a rotation whose fresh live file is already past the old offset by the next 500 ms poll (roughly a multi-MiB/s trace storm): the read then silently starts mid-file and drops the head of the fresh log. A version/id check — compare creation time, or fingerprint the first line against the previous file's first line — would make this exact. Rare and diagnostic-only, so non-blocking.
| } | ||
|
|
||
| /// Draws the filter over the tail again, and follows the end unless paused. | ||
| fn refresh(&mut self, cx: &mut Context<Self>) { |
There was a problem hiding this comment.
💡 Suggestion: While paused, refresh() recomputes visible from self.tail, which already contains the lines that arrived during the pause (they bypassed the snapshot only in take()). Editing the search text, level or source while paused therefore reveals pending lines while paused_at still says they are unconsumed. Clamping visible to seq <= paused_at while paused would make the hold-still promise hold for filter edits too.
| /// set the variable, so inheritance alone would give mediad and login a different one. The | ||
| /// ceiling goes too: a stored Verbose logging lives in chatd's settings, which a helper never | ||
| /// reads, so a mediad started after it was turned on starts at debug. | ||
| pub fn share_session(command: &mut std::process::Command) { |
There was a problem hiding this comment.
💡 Suggestion: The new TINYCLIENT_LOG_LEVEL env var set by share_session currently has no test — share_session_puts_the_id_on_the_child only asserts TINYCLIENT_SESSION, and since unit tests share process state nothing else would catch a regression here. One assertion on get_envs() when a guard is installed would do.
| pub source: Option<Source>, | ||
| /// The most detailed level shown; Debug includes trace. | ||
| pub level: Level, | ||
| /// Shown only if the line holds these words, in any case. Empty keeps everything. |
There was a problem hiding this comment.
💡 Suggestion: contains_folded folds ASCII bytes only, so the doc here slightly overstates the case-insensitivity for non-ASCII search terms. Either narrow the doc to ASCII, or fold via char::to_lowercase with the ASCII fast path kept for the common case.
… a little later. Windows CI downloaded the seeded avatar while Discord was still offline, then the READY avatar after connect, so the own-picture test saw two fetches. macOS CI paused a one-second file at 602 ms, two milliseconds outside the old 600 ms window.
Until now the only way to see logs was Settings › About › Save debug log and then opening files. This adds a window that shows them as they're written.
Logs window. Open it from Help › Logs, Ctrl+Shift+L (⇧⌘L on macOS), or View logs in Settings › About.
tinyclient.log,chatd.logandmediad.log. There are tabs for each process and an All tab that interleaves them.log_policy::render/redact.Verbose logging is a switch in the Logs window and a new row in Settings › About (B4). It reuses the existing stored
log_levelsetting, so the protocol needs no newRequest:Change::Settings.MediaRequest::LogLevel { level }to a running call's or mic test's mediad. This is a new variant on chatd's private stdin protocol with mediad, which ships in the same build.share_sessionputsTINYCLIENT_LOG_LEVELon helpers, so a mediad started later begins at the right level.LogGuard::applywrites a "log level is now debug" line to the file whenever the level changes, so the switch visibly takes effect in each process.All of this happens without a restart, and the setting is kept for the next start. Turning it off clears the row. If
TINYCLIENT_LOG_LEVELalone would keep the level at debug, it storesinfoinstead.Docs updated:
docs/logging.md, and B2/B4 indocs/product/09-settings.md.Tests
log_tail.rs: level parsing, line splitting, the ring bound, filters, and following a file (including rotation and the first read from the middle of a file). Also tests forlevel_word/verboseand theLogLevelframe.cargo test -p protocol,-p tinyclient(with and withoutgui),-p mediad --liband-p chatdall pass. The one exception isinvite_ipc::second_launch_delivers_to_the_primary, which fails here only because an installed client holds the pipe.Checked by hand on Windows with a debug build and a scratch
TINYCLIENT_DIR:🤖 Generated with Claude Code