Step-by-step visual guidance + fix 22 codebase audit issues#120
Open
HalxDocs wants to merge 1 commit into
Open
Step-by-step visual guidance + fix 22 codebase audit issues#120HalxDocs wants to merge 1 commit into
HalxDocs wants to merge 1 commit into
Conversation
Feature: step-by-step 'show me how' guidance - StepByStepGuide.swift: GuidanceStep model, StepByStepGuide parser, StepAdvanceCommand enum for voice-controlled step advancement - CompanionManager.swift: Full guide lifecycle — start, advance, cancel, finish. Preamble TTS with timeout safety, per-step voiceState tracking - OverlayWindow.swift: Step progress bar, step instruction overlay, per-step coordinate mapping for cursor pointing - 30+ tests covering parsing, edge cases, lifecycle states Fix 22 codebase audit issues: Critical: - OverlayWindow use-after-free crash from asyncAfter blocks after view teardown — added isViewActive guard flag checked in all 6 timer/asyncAfter callbacks. All closures now use [weak self]. - OverlayWindow timer leak — welcome animation timer now stored in @State and invalidated in onDisappear. - OverlayWindow fade race — guard at top of fadeOutAndHideOverlay prevents re-entrant animation groups. - ClaudeAPI proxy URL force-unwrap — init is now failable (init?). High: - ClaudeAPI leaked URLSession — added deinit with invalidateAndCancel(). - ClaudeAPI NSLock no defer — added defer { unlock() } right after lock(). - ClaudeAPI silent SSE parse drops — separated UTF-8 vs JSON failure logging so malformed events are not silently swallowed. - ClaudeAPI partial text loss — onTextChunk now sends individual deltas instead of full accumulated text. - ElevenLabsTTSClient force-unwrap + missing deinit — init? + deinit. - BuddyDictationManager audio thread data race — audio tap callback now dispatches to @mainactor via Task { @mainactor in }. - BuddyDictationManager session cancellation timing — sessionGeneration counter; callbacks check it to ignore stale results from preempted sessions. - CompanionManager stuck voice state on step TTS failure — cancelStepByStepGuide now sets voiceState = .idle. - CompanionManager preamble no timeout — 30-second safety timeout on while-isPlaying loop. - CompanionManager unnecessary cancel — guarded with if isStepByStepActive. Medium: - MenuBarPanelManager stale click-outside dismissal — panelShowGeneration counter; asyncAfter block checks it to avoid hiding a re-shown panel. - CompanionResponseOverlay redundant Task at 60fps — removed unnecessary Task { @mainactor } hop from cursor tracking timer. - CompanionScreenCaptureUtility NSScreen lookup failure on virtual displays — guard let skip instead of incorrect CG→AppKit coordinate fallback. Low: - GlobalPushToTalkShortcutMonitor use-after-free in CGEventTap callback — isRunning flag checked in the C callback before accessing self.
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
1 participant
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.
Summary
This PR adds step-by-step "show me how" visual guidance and fixes 22 issues from a comprehensive codebase audit — crashes, data races, memory leaks, and patterns that would manifest as production bugs. No behavioral changes to existing features.
✨ Step-by-Step Visual Guidance
Claude can return
[GUIDE:N]blocks for "how do I..." questions. Each step has an instruction and optional pointing coordinates — the blue cursor buddy flies to the relevant UI element while speaking the instruction.New:
leanring-buddy/StepByStepGuide.swift(166 lines) —GuidanceStep,StepByStepGuide,StepAdvanceCommand,StepByStepGuideParserCompanionManager.swift: +247 lines — Full lifecycle:
startStepByStepGuide,advanceToNextStep,finishStepByStepGuide,cancelStepByStepGuide,executeStep. Per-step state tracking with generation counter for TTS coordination. Preamble TTS with 30s timeout safety.OverlayWindow.swift: +57 lines — Step progress bar, step instruction overlay, per-step coordinate mapping with bezier-arc cursor flight
Tests: 30+ cases in
leanring_buddyTests.swift(+393 lines) — parsing, commands, lifecycle, cancellation, boundary conditions🔧 22 Audit Fixes
Critical (crash on teardown)
asyncAfterblocks touch@Stateafter view disappears. AddedisViewActiveguard flag in all 6 timer/asyncAfter callbacks with[weak self].@State welcomeAnimationTimer, invalidated inonDisappear.fadeOutAndHideOverlaynot re-entrant. Addedguard !overlayWindows.isEmpty.URL(string:)!crash. Madeinit?failable.High (data races, memory leaks, stuck state)
defer { unlock() }.deinit { invalidateAndCancel() }.try? JSONSerializationswallows parse errors. Added diagnosticprint().onTextChunksent full accumulated text. Now sends individual deltas.init?+deinit { invalidateAndCancel() }.activeTranscriptionSessionaccessed from background without MainActor isolation. Dispatches viaTask { @MainActor in }.sessionGenerationcounter checked by all callbacks.sessionGenerationnot incremented inresetSessionState. Fixed.cancelStepByStepGuidedidn't resetvoiceState. AddedvoiceState = .idle.while isPlayingspins forever. Added 30s timeout.cancelStepByStepGuide()called unconditionally. Guarded withif isStepByStepActive.Medium (races, performance, incorrect behavior)
asyncAfter(0.3s)fires after panel re-shown. AddedpanelShowGenerationcheck.Task { @MainActor }per frame. Removed hop — timer already on main runloop.guard let nsScreenwith skip.Low
CGEventTapCallBackaccesses deallocated self. AddedisRunningflag checked in callback.Testing
xcodebuild— preserves TCC permissions)Notes
leanring_buddytypo is intentional/legacyonChangewarning in OverlayWindow.swift are not addressed