Skip to content

Fix analyze/resub timing out on long videos regardless of progress - #2

Open
aangelinsf wants to merge 2 commits into
leeyc09:mainfrom
aangelinsf:fix/analyze-idle-timeout
Open

aangelinsf wants to merge 2 commits into
leeyc09:mainfrom
aangelinsf:fix/analyze-idle-timeout

Conversation

@aangelinsf

Copy link
Copy Markdown

Problem

PythonBridge.call() used a flat wall-clock timeout (600s, i.e. 10 minutes, for both analyze and resub) 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:

  • Track lastActivityAt, updated every time a line is parsed from the subprocess — this covers both progress notifications and RPC responses, using plumbing the app already has (ProgressInfo/currentProgress).
  • The watchdog task now polls every 5s instead of sleeping once for the full timeout, and only expires the request if timeout seconds have elapsed since the last activity, not since the request started.
  • A request still gets cancelled promptly if the Python side genuinely hangs (no progress or response for the full timeout window) — that's the actual failure mode a timeout should catch.

No changes to AnalysisService.swift — the existing timeout: 600 becomes 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):

  • Before fix: GUI failed with "Request timed out" at the 10-minute mark.
  • After fix: GUI completed successfully in ~11 minutes (matching CLI timing for the same file via 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:

  • Added lastActivityAt: Date tracking, bumped at the top of handleResponse(_:).
  • Rewrote the call() watchdog from a single Task.sleep(for: .seconds(timeout)) into a polling loop checking idle time via a new watchdogStatus(for:) MainActor helper.

André Angelantoni 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.
@aangelinsf

Copy link
Copy Markdown
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).

  • Extracted the expiry decision into a pure, nonisolated PythonBridge.isIdleExpired(lastActivityAt:now:timeout:).
  • Added a SilenciAppTests XCTest target (new .testTarget in Package.swift) covering: normal activity within the timeout window, the exact boundary, a genuine stall (confirms the fix doesn't remove the original safety net), and the specific regression this PR targets — 20 simulated progress notifications spaced 100s apart (2000s total, well past the old flat 600s cutoff) never triggering expiry.
  • swift test: 6/6 passing. Release build (build-release.sh) still succeeds.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant