feat(logging): structured logging via tracing across all binaries - #11
Open
jakob1379 wants to merge 1 commit into
Open
feat(logging): structured logging via tracing across all binaries#11jakob1379 wants to merge 1 commit into
jakob1379 wants to merge 1 commit into
Conversation
The desktop UI surfaced backend warnings that never reached the terminal: emit_activity() only invoked the UI callback, and the CLI's bare env_logger::init() defaulted to `error`, hiding every existing log:: site. Replace env_logger with tracing + tracing-subscriber behind a shared logging::init(). RUST_LOG sets the filter (default info; an empty or unparseable value falls back to info rather than silencing the process), DIFFCORE_LOG_FORMAT=json switches renderer, and DIFFCORE_LOG_FILE redirects to a file. Output prefers stderr whenever it is a terminal; the desktop app falls back to ~/.diffcore/desktop.log only when it is not, since Finder and .desktop launches discard stderr. ANSI is emitted only to a real terminal and honours NO_COLOR. Existing log:: call sites and dependency logs bridge in via tracing-log, so no call site had to be rewritten. Route activity through emit_activity_with() so the UI callback and the logging path can no longer diverge; task-locals do not cross tokio::spawn, so stream collectors pass the callback explicitly. Close the channels that showed a user something while logging nothing: JobHandle::fail, CommandError's IPC serialization, codex error items and unrecognised failure events (which logged at info as "Completed error" with no detail, and now carry the nested error message). Redact api keys from activity messages before they reach persistent logs. Doing so exposed two bugs in redact_api_keys, now fixed: it byte-sliced at 500 bytes while activity messages cap at 180 *chars*, panicking mid-codepoint on non-ASCII output and killing the whole run via a JoinError; and `sk-` matched inside ordinary words (task-, risk-, disk-), corrupting them while `break` skipped real keys later in the same line. Note update.payload is still unredacted -- it does not reach the logs, only the UI and SSE stream. Filterable targets: `activity` for backend events, `ir_cache` for the IR cache (replacing the bespoke DIFFCORE_CACHE_DEBUG env gate), `refinement` for repair warnings, `command` for IPC errors. tracing-subscriber sits behind a default-off `logging` feature so library consumers do not link the subscriber stack.
jakob1379
force-pushed
the
t3code/add-structured-logging
branch
from
August 28, 2026 11:44
d16144b to
aa551e2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces
env_loggerwithtracing+tracing-subscriberbehind a sharedlogging::init(), so the warnings the desktop UI already displays also reach the terminal.The bug
emit_activity()only invoked the UI callback, and the CLI's bareenv_logger::init()defaulted toerror, hiding every existinglog::call site.Activity now routes through
emit_activity_with(), where the callback is a parameter rather than ambient state. Task-locals don't crosstokio::spawn, so the stream collectors captured the callback up front and had diverged from the logging path — in the desktop app, only the two pre-spawn "Launching…" events ever logged.Configuration
RUST_LOGsets the filter (defaultinfo). Empty or unparseable falls back toinforather than silencing the process.DIFFCORE_LOG_FORMAT=jsonswitches to line-delimited JSON.~/.diffcore/desktop.logonly when it is not — GUI bundles discard stderr (windows_subsystem = "windows"in release; Finder/.desktoplaunches drop it).DIFFCORE_LOG_FILEoverrides either way.NO_COLOR.activity(backend events),ir_cache(replacing the bespokeDIFFCORE_CACHE_DEBUGgate),refinement,command.Channels that showed the user something but logged nothing
JobHandle::fail,CommandError's IPC serialization (the funnel ~30 tauri commands report failure through), and codex error items — which logged atinfoas"Completed error"with no detail, and now carry the nested error message aterror.Two pre-existing
redact_api_keysbugsBoth became reachable by routing activity messages through redaction:
It byte-sliced at 500 bytes while activity messages cap at 180 chars. 183 CJK chars is 549 bytes, so it panicked mid-codepoint. The panic lands inside a
tokio::spawned collector, surfacing as a misleading"Failed to join claude stdout task"and discarding the entire buffered LLM response.sk-matched inside ordinary words, andbreakthen skipped real keys later in the same line:Both fixed at the root with regression tests. Note
update.payloadremains unredacted — it does not reach the logs, only the UI and SSE stream.Notes
tracing-subscribersits behind a default-offloggingfeature, so library consumers ofdiffcore-coredon't link the subscriber stack (cargo tree -p diffcore-core -e normalshows zero matches).log::call sites and dependency logs bridge in viatracing-log. No call site was rewritten.diffcore analyze | jqstill works.Testing
1706 lib tests plus all integration suites and the tauri web suite pass;
nix buildis green. The regression test drives the realcollect_claude_streamwith a callback installed and was verified to fail (empty log output) against the pre-fix code.Known follow-ups, deliberately out of scope
codex_cli.rsdrops non-JSON lines, so codex raw stderr produces no activity where claude's produces a WARN. Pre-existing.~/.diffcore/desktop.loghas no rotation once it is used.Clippy
Measured against the merge-base with a matching toolchain:
No new findings. The three removed errors are the
eprintln!calls inpipeline.rsthat violated the crate's own#![deny(clippy::print_stderr)]. CI runs onlycargo test --workspace --locked, so those deny attributes were never enforced — adding a clippy job would need the 7 remaining pre-existing errors addressed first, so I have left that out of this PR.