Fix analyze/resub timing out on long videos regardless of progress - #2
Open
aangelinsf wants to merge 2 commits into
Open
aangelinsf wants to merge 2 commits into
aangelinsf wants to merge 2 commits into
Conversation
added 2 commits
August 27, 2026 11:16
PythonBridge.call() used a flat wall-clock timeout (600s for analyze/resub) that fired unconditionally once elapsed, even while the Python subprocess was actively emitting progress notifications. This makes long-video analysis (e.g. 2+ hour recordings) fail with "Request timed out" well before the VAD/ASR/ForcedAligner pipeline can finish, regardless of how far along it actually is. Replace the one-shot deadline with an idle watchdog: the timeout now resets on every parsed line from the subprocess (progress notification or RPC response), and only fires if the subprocess goes silent for the full timeout duration. This uses the progress-notification plumbing the app already has, rather than raising the fixed cap and risking the same failure on even longer input. Verified against a real 2h8m video: unpatched GUI failed with "Request timed out" at the 10-minute mark; patched GUI completed successfully in ~11 minutes (matching CLI timing for the same file), producing a valid FCPXML with word-level subtitles and silence cuts.
The repo had no test target or CI, so the prior commit's fix was only verified manually. Extract the idle-expiry decision into a pure, nonisolated static function (PythonBridge.isIdleExpired) and add an XCTest target covering it directly — no subprocess required, runs in milliseconds. Covers: normal activity within the timeout window, the exact timeout boundary, a genuine stall (still expires — the fix doesn't remove the safety net), and the actual regression this PR targets: 20 progress notifications spaced 100s apart (2000s total, well past the old flat 600s cutoff) never triggering expiry since each one resets the clock. `swift test`: 6/6 passing. Release build unaffected.
Author
|
Added unit test coverage for the idle-watchdog logic (the repo had no test target/CI, so the original commit was only verified manually against the real 2h8m video).
Happy to adjust test style/location if you have a preferred convention — this is the first test target in the repo, so I picked the most minimal/standard SwiftPM layout. |
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.
Problem
PythonBridge.call()used a flat wall-clock timeout (600s, i.e. 10 minutes, for bothanalyzeandresub) that fires unconditionally once elapsed — even while the Python subprocess is actively emitting progress notifications over the JSON-RPC bridge.In practice this means analyzing any video whose VAD + Qwen3-ASR + ForcedAligner pipeline legitimately takes longer than 10 minutes fails with "Request timed out," regardless of how far along the analysis actually is. This hits multi-hour recordings (podcasts, lectures, long-form tutorials) hard, since per-segment ASR + alignment time scales with speech duration.
Fix
Replace the one-shot deadline with an idle watchdog:
lastActivityAt, updated every time a line is parsed from the subprocess — this covers bothprogressnotifications and RPC responses, using plumbing the app already has (ProgressInfo/currentProgress).timeoutseconds have elapsed since the last activity, not since the request started.No changes to
AnalysisService.swift— the existingtimeout: 600becomes the idle threshold, which is still a sensible default (10 minutes of total silence from the subprocess is a reasonable "something's actually stuck" signal).Testing
Verified against a real 2h8m48s video (1675 speech segments detected by VAD):
silence-cutter cut), loaded 1675 segments into the word-level editor, and exported a valid FCPXML (xmllint-clean, 1130 asset-clips, 4176 title elements, full duration matched to the source).Diff summary
SilenciApp/Sources/Services/PythonBridge.swift:lastActivityAt: Datetracking, bumped at the top ofhandleResponse(_:).call()watchdog from a singleTask.sleep(for: .seconds(timeout))into a polling loop checking idle time via a newwatchdogStatus(for:)MainActor helper.