fix(desktop): crash in Plan & Usage settings + OCR array bounds (#6506, #5151)#6540
fix(desktop): crash in Plan & Usage settings + OCR array bounds (#6506, #5151)#6540
Conversation
#5151) Plan & Usage crash (#6506): - Filter out plans/prices with empty IDs before dictionary merge - Reserve dictionary capacity to avoid reallocation during merge - Prevents assertion failure in _NativeDictionary.merge on malformed API data OCR crash (#5151): - Eagerly snapshot all text, confidence, and bounding box data from Vision's bridged NSArray in a single pass before building OCRTextBlocks - Copies String values out of bridged objects immediately to avoid accessing deallocated bridged array storage during iteration - Reserve array capacity for blocks and text lines
Greptile SummaryFixes two macOS crash sites: a Confidence Score: 5/5Safe to merge — both fixes are defensive guards that only affect malformed or edge-case inputs, with no risk to the happy path. No P0 or P1 issues found. The two P2 comments are cosmetic (a slightly under-sized No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[mergePlanCatalog called] --> B[Filter empty-ID plans from primary & fallback]
B --> C[reserveCapacity on mergedById dict]
C --> D[Insert fallback plans into dict]
D --> E{Plan ID in dict?}
E -- Yes --> F[Merge plan fields + call mergePrices]
E -- No --> G[Insert primary plan]
F --> H[Return merged array]
G --> H
subgraph OCR Pipeline
I[VNRecognizeTextRequest callback] --> J[Array copy of bridged NSArray observations]
J --> K[Iterate: extract text+confidence+box into RawOCR structs]
K --> L[Build OCRTextBlock array from RawOCR]
L --> M[Return OCRResult]
end
Reviews (1): Last reviewed commit: "fix(desktop): crash in Plan & Usage sett..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let safeFallback = fallback.filter { !$0.id.isEmpty } | ||
|
|
||
| var mergedById: [String: SubscriptionPlanOption] = [:] | ||
| mergedById.reserveCapacity(max(safePrimary.count, safeFallback.count)) |
There was a problem hiding this comment.
reserveCapacity underestimates worst-case size
max(safePrimary.count, safeFallback.count) reserves capacity for the larger of the two slices, but the merged dictionary can hold up to safePrimary.count + safeFallback.count entries when all IDs are disjoint. The same pattern repeats in mergePrices (line 102). The practical impact is limited since plan catalogs typically share IDs, but a sum would be more accurate and eliminate any potential reallocation.
| mergedById.reserveCapacity(max(safePrimary.count, safeFallback.count)) | |
| mergedById.reserveCapacity(safePrimary.count + safeFallback.count) |
| for i in 0..<count { | ||
| let observation = safeObservations[i] | ||
| let candidates = observation.topCandidates(1) | ||
| guard !candidates.isEmpty, let candidate = candidates.first else { continue } | ||
| guard let candidate = candidates.first else { continue } | ||
| let box = observation.boundingBox | ||
| guard box.origin.x.isFinite, box.origin.y.isFinite, | ||
| box.width.isFinite, box.height.isFinite else { continue } | ||
| rawResults.append(RawOCR( | ||
| text: String(candidate.string), | ||
| confidence: candidate.confidence, | ||
| box: box | ||
| )) | ||
| } |
There was a problem hiding this comment.
for-in
safeObservations is a native Swift Array (created via Array(observations)), so iterating with for i in 0..<count and then subscripting offers no safety advantage over for observation in safeObservations. The original crash came from the bridged NSArray storage, which is already neutralised by the Array(...) copy. A for-in loop is more idiomatic and avoids the extra count capture:
| for i in 0..<count { | |
| let observation = safeObservations[i] | |
| let candidates = observation.topCandidates(1) | |
| guard !candidates.isEmpty, let candidate = candidates.first else { continue } | |
| guard let candidate = candidates.first else { continue } | |
| let box = observation.boundingBox | |
| guard box.origin.x.isFinite, box.origin.y.isFinite, | |
| box.width.isFinite, box.height.isFinite else { continue } | |
| rawResults.append(RawOCR( | |
| text: String(candidate.string), | |
| confidence: candidate.confidence, | |
| box: box | |
| )) | |
| } | |
| for observation in safeObservations { | |
| let candidates = observation.topCandidates(1) | |
| guard let candidate = candidates.first else { continue } | |
| let box = observation.boundingBox | |
| guard box.origin.x.isFinite, box.origin.y.isFinite, | |
| box.width.isFinite, box.height.isFinite else { continue } | |
| rawResults.append(RawOCR( | |
| text: String(candidate.string), | |
| confidence: candidate.confidence, | |
| box: box | |
| )) | |
| } |
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!
1 participant
What
Fixes two crashes in the macOS desktop app:
Plan & Usage settings crash (#6506)
Root cause:
SubscriptionPlanCatalogMerger.mergecrashes withEXC_BREAKPOINTin_NativeDictionary.mergewhen the backend returns plans or prices with empty IDs, triggering a Swift dictionary precondition failure during body evaluation.Fix:
Crash trace: Main thread →
SettingsContentView.mergePlanCatalog→_NativeDictionary.mergeassertion failureOCR array bounds crash (#5151)
Root cause: Vision framework's bridged
NSArraystorage forVNRecognizedTextObservationresults can be invalidated during iteration, causing_objectAtassertion failures even with the existingArray(observations)defensive copy.Fix:
RawOCRstructStringvalues out of bridged objects immediately viaString(candidate.string)OCRTextBlockarray from already-copied native Swift dataCrash trace: Thread 9 →
RewindOCRService.extractTextWithBounds→__SwiftNativeNSArrayWithContiguousStorage._objectAtassertion failureTesting
swift buildpasses ✅Closes #6506
Relates to #5151