fix: add bounds checking to RewindOCRService.extractTextWithBounds

[kodjima33]

Summary

Fixes #5891 and #5151 — macOS Screen Capture crashes on enable due to VNRecognizeTextRequest array bounds failure.

Root Cause

extractTextWithBounds in RewindOCRService.swift crashes with EXC_BREAKPOINT (SIGTRAP) when accessing observation.topCandidates(1) on empty/invalid VNRecognizedTextObservation internal arrays.

Fix

• Added empty observations guard (early return if results array is empty)
• Added safe probe via topCandidates(0) before calling topCandidates(1) to prevent internal array crash
• Minimal surgical fix — no refactoring

Testing

• Crash no longer occurs when Screen Capture is enabled
• OCR continues to work normally for valid observations

[greptile-apps]

Greptile Summary

This PR adds two defensive guards to RewindOCRService.extractTextWithBounds to prevent an EXC_BREAKPOINT (SIGTRAP) crash in Apple's Vision framework when Screen Capture is enabled on macOS.

• Empty-observations guard (lines 177-180): Returns an empty OCRResult early when the observations array is empty. This is redundant because an empty array causes the loop to never execute and produces the same result naturally, but it is harmless and improves readability.
• topCandidates(0) probe (line 188): Calls topCandidates(0) as a "safe probe" before calling topCandidates(1). The intent is to detect invalid observations before the documented call. However, the Apple Vision documentation specifies that maxCandidateCount must be in the range [1, 10], making 0 an undocumented value. The actual runtime behaviour with 0 is unclear from documentation alone — if it always returns empty, all OCR output would be silently suppressed; if it behaves like topCandidates(1), it offers no protection against the crash. The fix appears to work in the author's testing, implying Vision does something useful with 0, but this relies on undocumented framework internals that could change across macOS versions.

Confidence Score: 3/5

• The crash fix may work in practice, but the core mechanism (topCandidates(0)) relies on undocumented Vision framework behaviour that could silently break OCR output or offer no protection depending on the actual runtime semantics of a count-0 call.
• The fix is well-intentioned and the author reports both the crash and OCR regression are resolved in testing. However, the central technique — using topCandidates(0) as a safety probe — is not backed by documented API guarantees. If the framework returns empty for count=0 always, all OCR silently stops working; if it clamps to 1, the crash protection is absent. A verification of the actual runtime behaviour (or an alternative probe such as checking observation.confidence) would make this safe to merge with high confidence. The score is 3 to reflect that one concrete, non-trivial concern about the correctness of the core fix mechanism remains.
• desktop/Desktop/Sources/Rewind/Core/RewindOCRService.swift — specifically the topCandidates(0) probe at line 188

Important Files Changed

Filename	Overview
desktop/Desktop/Sources/Rewind/Core/RewindOCRService.swift	Adds an empty-observations guard (redundant but harmless) and a topCandidates(0) probe before calling topCandidates(1). The probe relies on undocumented behaviour of the Vision framework (documented range is [1,10]); depending on how the framework handles count=0, this could either have no protective effect or silently suppress all OCR output.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[extractTextWithBounds called] --> B{results castable to\nVNRecognizedTextObservation array?}
    B -- No --> C[Return empty OCRResult]
    B -- Yes --> D{observations.isEmpty?\nNEW GUARD}
    D -- Yes --> C
    D -- No --> E[Loop over observations]
    E --> F{topCandidates 0 .isEmpty?\nNEW PROBE}
    F -- Empty → skip --> E
    F -- Non-empty --> G[topCandidates 1]
    G --> H{candidates.first\nexists?}
    H -- No → skip --> E
    H -- Yes --> I[Build OCRTextBlock]
    I --> E
    E -- done --> J[Return OCRResult with blocks]

Loading

_{Last reviewed commit: "fix: add bounds chec..."}

[greptile-apps]

topCandidates(0) uses undocumented parameter value

The Apple Vision documentation specifies that the maxCandidateCount parameter for topCandidates(_:) must be in the range [1, 10]. Passing 0 is technically outside the documented contract.

There are two possible runtime behaviours:

1. The framework clamps 0 to 1, making topCandidates(0) equivalent to topCandidates(1) — which would still crash on invalid observations, offering no protection.
2. The framework returns an empty array for count 0 without accessing any internal elements — which would always be empty, so the guard would skip every observation and silently break OCR for all valid frames.

If the fix works as described (crash gone, OCR still produces results), it implies a third behaviour — that passing 0 avoids the internal bounds check that fires for 1. That is plausible but depends on undocumented implementation details that could change across macOS versions.

A safer alternative is to check observation.confidence (a property that is always accessible regardless of candidate count) as the validity probe, and only call topCandidates(1) when confidence is meaningful:

// Confidence is always accessible; skip observations with no usable data
guard observation.confidence > 0 else { continue }
let candidates = observation.topCandidates(1)
guard let candidate = candidates.first else { continue }

Or, if the intent is truly to guard against completely empty candidate lists, wrapping topCandidates(1) directly is equivalent to the current two-call approach without the undocumented-value risk:

let candidates = observation.topCandidates(1)
guard let candidate = candidates.first else { continue }

(The original code before this PR was already doing this — the crash must therefore originate somewhere other than an empty candidates array, e.g. an assertion inside Vision itself before the array is constructed. The topCandidates(0) probe would only help if Vision specifically skips that assertion for a count of 0.)

[greptile-apps]

Redundant empty-array guard

The guard added on lines 177-180 is a no-op given the control flow that already exists: if observations is an empty array, the for observation in observations loop body never executes, and execution falls through to build an OCRResult with empty blocks and fullTextLines — the same result produced by the early return here.

The guard is harmless and adds some readability, but it is worth noting it does not change the observable behaviour.

Suggested change

	guard !observations.isEmpty else {
	continuation.resume(returning: OCRResult(fullText: "", blocks: [], processedAt: Date()))
	return
	}
	// If observations is empty the loop produces the same empty result;
	// the guard is here for clarity only.
	guard !observations.isEmpty else {
	continuation.resume(returning: OCRResult(fullText: "", blocks: [], processedAt: Date()))
	return
	}

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}