Add unified debug logging across input pipeline - #12
Conversation
Introduce SwitchFixLog (subsystem com.switchfix, [SwitchFix] message prefix, public values) and wire it through the whole pipeline: per-keystroke capture, buffer transitions, word flushes, detection verdicts incl. suppression reasons, correction apply/revert/selection, layout switches, focus/app changes, permissions and dictionary loading. Replace scattered NSLog calls with the unified logger so everything is filterable via: log stream --level debug --style compact --predicate 'subsystem == "com.switchfix"'
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95bb2b8bbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| /// Default level: visible in plain `log stream` with no --level flag. | ||
| public func notice(_ message: String) { | ||
| logger.notice("[SwitchFix] \(message, privacy: .public)") |
There was a problem hiding this comment.
Callers such as InputEngine.runDetection and LayoutDetector.checkBuffer include users' original and converted words in these messages, but this helper marks the entire preformatted string as .public. Consequently, text typed in any allowed non-secure application—including messages, searches, and document content—is emitted unredacted to macOS unified logging, with corrected words logged at the default notice level. Keep user-derived text private or omit/hash it while exposing only diagnostic metadata.
Useful? React with 👍 / 👎.
|
/ocr |
|
🔍 OpenCodeReview found 9 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| case .invalidate(let reason): | ||
| resetDetectorState() | ||
| logger.debug("buffer invalidated reason=\(String(describing: reason), privacy: .public)") | ||
| logger.debug("buffer invalidated reason=\(String(describing: reason))") |
There was a problem hiding this comment.
Dropping privacy: .public means dynamic fields like reason render as <private> in release builds, so field diagnostics collected from users will be redacted exactly where the new unified-debug-logging effort needs visibility. It also diverges from the SwitchFixLog convention used elsewhere in this change, which marks all values public. Annotate non-sensitive diagnostic enums/values with privacy: .public.
Suggestion:
| logger.debug("buffer invalidated reason=\(String(describing: reason))") | |
| logger.debug("buffer invalidated reason=\(String(describing: reason), privacy: .public)") |
| if let result { | ||
| SwitchFixLog.detector.notice( | ||
| "detect '\(result.originalWord)' -> '\(result.convertedWord)' source=\(result.sourceLayout.rawValue) target=\(result.targetLayout.rawValue) switch=\(result.shouldSwitchLayout) ms=\(Double(duration) / 1_000_000.0)" | ||
| ) |
There was a problem hiding this comment.
User-typed words now flow into durable logs: SwitchFixLogger marks its entire message privacy: .public (readable in Console.app/log stream per its own doc comment) and .notice entries are persisted to the unified logging store on disk. For a keyboard-monitoring tool, this durably records typed text — search queries, message drafts, even credentials entered into fields not detected as secure — on user machines. Prefer keeping the notice-level line free of word content (shape/timing only) and emitting the actual words only at .debug (memory-only) behind an explicit opt-in.
Suggestion:
| if let result { | |
| SwitchFixLog.detector.notice( | |
| "detect '\(result.originalWord)' -> '\(result.convertedWord)' source=\(result.sourceLayout.rawValue) target=\(result.targetLayout.rawValue) switch=\(result.shouldSwitchLayout) ms=\(Double(duration) / 1_000_000.0)" | |
| ) | |
| if let result { | |
| SwitchFixLog.detector.notice( | |
| "detect source=\(result.sourceLayout.rawValue) target=\(result.targetLayout.rawValue) switch=\(result.shouldSwitchLayout) ms=\(Double(duration) / 1_000_000.0)" | |
| ) | |
| SwitchFixLog.detector.debug( | |
| "detect '\(result.originalWord)' -> '\(result.convertedWord)'" | |
| ) |
| guard cancelReason == nil else { | ||
| logger.debug("correction cancelled reason=\(cancelReason!) word='\(result.originalWord)'") | ||
| return | ||
| } |
There was a problem hiding this comment.
cancelReason! is currently unreachable-failure because the preceding chain guarantees non-nil, but the invariant is purely positional: reordering branches or editing the guard later turns this into a runtime crash on a user-facing correction path. Prefer optional binding over force unwrap here.
Suggestion:
| guard cancelReason == nil else { | |
| logger.debug("correction cancelled reason=\(cancelReason!) word='\(result.originalWord)'") | |
| return | |
| } | |
| if let cancelReason { | |
| logger.debug("correction cancelled reason=\(cancelReason) word='\(result.originalWord)'") | |
| return | |
| } |
|
|
||
| // Skip if the word contains mixed scripts (both Latin and Cyrillic) | ||
| if containsMixedScripts(word) { | ||
| SwitchFixLog.detector.debug("mixed scripts, skipping '\(word)'") |
There was a problem hiding this comment.
These new statements write raw user-typed text (the keystroke-derived word buffer) into the unified logging system with .public privacy, so typed words become plainly readable in Console.app, log stream, and log collect exports. The same pattern applies to the other three additions in this diff ('\(currentLanguage.rawValue): '\(word)', "suppressed short word '(word)' -> '(finalWord)'", "unknown word '(word)'"). Since this app observes arbitrary global typing — which can include usernames/passwords entered outside secure fields — consider redacting the dynamic content (e.g., log word length, first character + hash, or a truncated form) instead of the verbatim word, or gate verbose content logging behind an explicit user opt-in. Note the SwitchFixLogger wrapper hardcodes privacy: .public and accepts a plain String, so redaction must happen at the call site (or a .private variant must be added to the logger).
Suggestion:
| SwitchFixLog.detector.debug("mixed scripts, skipping '\(word)'") | |
| SwitchFixLog.detector.debug("mixed scripts, skipping \(word.count)-char word") |
| import CoreGraphics | ||
| import Foundation | ||
| import os | ||
| import Utils |
There was a problem hiding this comment.
import Utils was added, but this class still logs through its own Logger(subsystem: "com.switchfix", category: "correction") instead of the new unified SwitchFixLog.corrector. As a result these messages lack the "[SwitchFix]" prefix that SwitchFixLog documents as its filtering contract (eventMessage CONTAINS "[SwitchFix]"), and strings interpolated without privacy: .public are redacted as <private> in Console.app/log stream — so the newly added text/layout details are unreadable, defeating the point of the unified-logging change. Point logger at SwitchFixLog.corrector (its debug/info/notice methods are drop-in compatible with these call sites) and remove the hand-rolled Logger.
Suggestion:
| import Utils | |
| // Replace the local property: | |
| // private let logger = Logger(subsystem: "com.switchfix", category: "correction") | |
| private let logger = SwitchFixLog.corrector |
| logger.notice( | ||
| "correction APPLIED '\(plan.correctedText)' <- '\(plan.originalText)' deletes=\(plan.deleteCount) pid=\(plan.targetPID) layoutSwitch=\(plan.targetLayout?.rawValue ?? "none")" | ||
| ) |
There was a problem hiding this comment.
This notice (and the matching 'revert APPLIED' and 'selection paste' notices) embeds verbatim user-typed text. notice is visible in a plain log stream with no flags and is persisted in the unified log store, and once routed through SwitchFixLogger every dynamic value is forced privacy: .public — so arbitrary user input (names, emails, message drafts typed in non-secure fields) would land in persistent system logs readable by anything with log access. Note that with the current raw os.Logger the strings are incidentally <private>-redacted, so switching to SwitchFixLogger at notice level would actually be a privacy regression. Prefer debug for text-bearing diagnostics, or log only lengths/truncated hashes at notice.
Suggestion:
| logger.notice( | |
| "correction APPLIED '\(plan.correctedText)' <- '\(plan.originalText)' deletes=\(plan.deleteCount) pid=\(plan.targetPID) layoutSwitch=\(plan.targetLayout?.rawValue ?? "none")" | |
| ) | |
| logger.debug( | |
| "correction APPLIED '\(plan.correctedText)' <- '\(plan.originalText)' deletes=\(plan.deleteCount) pid=\(plan.targetPID) layoutSwitch=\(plan.targetLayout?.rawValue ?? "none")" | |
| ) |
| public static let app = SwitchFixLogger(category: "app") | ||
| public static let monitor = SwitchFixLogger(category: "monitor") | ||
| public static let engine = SwitchFixLogger(category: "engine") | ||
| public static let state = SwitchFixLogger(category: "state") |
There was a problem hiding this comment.
state is declared here but no call site uses SwitchFixLog.state (the engine still logs via its own local Logger(subsystem: "com.switchfix", category: "input-engine")). Either route InputEngine's messages through this category or remove the unused static so the category list matches actual usage.
| /// Every message is prefixed "[SwitchFix]" so it survives filtering with | ||
| /// `eventMessage CONTAINS "[SwitchFix]"`, and all dynamic values are logged | ||
| /// public so they are readable in Console.app and `log stream`. | ||
| public struct SwitchFixLogger { |
There was a problem hiding this comment.
This struct is used as process-wide statics (SwitchFixLog.app etc.) and is called from multiple threads (event-tap run loop, input/detection/correction queues, main thread). It is stateless, but it is not declared Sendable, so under Swift 6 / strict concurrency checking every cross-context use of these statics becomes a compile-time error. Since it only wraps an immutable Logger (which is itself Sendable), marking it Sendable is safe and future-proofs the shared statics.
Suggestion:
| public struct SwitchFixLogger { | |
| public struct SwitchFixLogger: Sendable { |
| /// Default level: visible in plain `log stream` with no --level flag. | ||
| public func notice(_ message: String) { | ||
| logger.notice("[SwitchFix] \(message, privacy: .public)") | ||
| } |
There was a problem hiding this comment.
Marking the entire interpolated message privacy: .public writes user-typed content in plaintext to the unified log store. This logger is the pipeline-wide default, and call sites log user text at notice/error level (e.g. "word flushed ''", "correction APPLIED '' <- ''", "selection paste ''"), which is persisted by default and readable via log stream/Console.app without special privileges. Since the app deliberately skips secure fields but cannot know what users type elsewhere, consider keeping dynamic content private (os_log redacts it by default) or masking user words, reserving .public for non-sensitive fields (pids, key codes, durations) interpolated at call sites; or gate fully public logging behind a debug-only flag.
Suggestion:
| /// Default level: visible in plain `log stream` with no --level flag. | |
| public func notice(_ message: String) { | |
| logger.notice("[SwitchFix] \(message, privacy: .public)") | |
| } | |
| /// Default level: visible in plain `log stream` with no --level flag. | |
| public func notice(_ message: String) { | |
| logger.notice("[SwitchFix] \(message, privacy: .private)") | |
| } | |
| // Call sites that need a readable field should opt in per value, e.g.: | |
| // logger.notice("word flushed seq=\(sequence, privacy: .public) word='\(word)'") |
1 participant
Summary
SwitchFixLog(Sources/Utils/SwitchFixLog.swift): central os.Logger wrapper, subsystemcom.switchfix, categories per component, every message prefixed[SwitchFix], dynamic values logged public so they are readable in Console/log toolingKeyboardMonitor: event tap lifecycle (session/HID), tap resets/re-enables, failure diagnostics with permission statesInputEngine: per-keystroke capture metadata, buffer append/backspace state, word flushes, hotkey/revert requests, detection results with duration, correction planning with explicit cancel reasonsLayoutDetector: valid-word/unknown/mixed-script/suppressed-short decisions — answers "why wasn't my word corrected"TextCorrector: correction applied / revert applied / selection paste outcomes with before→after text and delete counts; undo skip reasonsInputSourceManager: layout switch success/failureAppDelegate: launch, monitoring start, frontmost app changes, layout changes, focus resolution, preference updatesNSLogcalls (Permissions, DictionaryLoader, PreferencesManager) to the unified loggerinstall.shprints the recommended log-stream command on completionHow to watch logs
Note:
--level debugis required for per-keystroke/debug output; without it only notice-level events (corrections applied, word flushes, layout changes) appear. The originaleventMessage CONTAINS "[SwitchFix]"predicate also works.Test plan
swift build -c releasepasses on top of latest masterlog streaminstall.sh