Skip to content

fix(apple-ai): wrap FoundationModels code in canImport check to fix CI build#1864

Open
peterfriese wants to merge 2 commits into
wwdc26-previewfrom
peterfriese/firebase-ai/fix-ci-build
Open

fix(apple-ai): wrap FoundationModels code in canImport check to fix CI build#1864
peterfriese wants to merge 2 commits into
wwdc26-previewfrom
peterfriese/firebase-ai/fix-ci-build

Conversation

@peterfriese

Copy link
Copy Markdown
Contributor

This PR wraps all new Apple Intelligence FoundationModels code pathways and navigation routes inside #if canImport(FoundationModels) compile-time checks. This allows the project to build successfully on older SDK targets (like Xcode 16.4/iOS 18.5) where the FoundationModels framework does not exist.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request integrates Apple's local Foundation Models (Apple Intelligence) with automatic cloud fallback into the Firebase AI Example app, introducing new screens and view models for 'Hybrid AI' and 'Vision ID' features, along with robust ML asset error handling. The code review provides several valuable recommendations to improve the implementation: using the .task(id:) modifier in VisionIDView to avoid race conditions during image loading, guarding against task cancellation in HybridAIViewModel and VisionIDViewModel to prevent redundant cloud fallback requests, clearing the error state when the error sheet is dismissed in FoundationModelsContainer, and hiding the default scroll content background of TextEditor in HybridAIView to correctly display the custom background color.

Comment on lines +54 to +62
.onChange(of: photosPickerItem) { oldItem, newItem in
Task {
if let data = try? await newItem?.loadTransferable(type: Data.self),
let image = UIImage(data: data) {
viewModel.selectedImage = image
viewModel.identifySelectedImage()
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Spawning an unstructured Task inside .onChange to load the image data asynchronously can lead to a race condition. If the user quickly selects multiple images in succession, the tasks may complete out of order, resulting in the wrong image being displayed and processed. Using the .task(id:) modifier is the idiomatic SwiftUI way to handle asynchronous work triggered by state changes, as it automatically cancels the previous task when the ID changes.

      .task(id: photosPickerItem) {
        guard let photosPickerItem else { return }
        if let data = try? await photosPickerItem.loadTransferable(type: Data.self),
           let image = UIImage(data: data) {
          viewModel.selectedImage = image
          viewModel.identifySelectedImage()
        }
      }

Comment on lines +60 to +69
} catch {
// Fall back to cloud model if local model fails
if error.isMLAssetUnavailable {
print("Local ML assets unavailable. Falling back to cloud model...")
} else {
print(
"Local model failed: \(error.localizedDescription). Falling back to cloud model..."
)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If the local model task is cancelled (for example, if the user cancels the active task or starts a new summarization), the respond call will throw a cancellation error. Catching this error and proceeding directly to the cloud fallback is incorrect, as it will trigger an unwanted cloud API request for a cancelled operation. Guarding against cancellation at the start of the catch block prevents this resource waste.

        } catch {
          guard !Task.isCancelled else { return }
          // Fall back to cloud model if local model fails
          if error.isMLAssetUnavailable {
            print("Local ML assets unavailable. Falling back to cloud model...")
          } else {
            print(
              "Local model failed: \(error.localizedDescription). Falling back to cloud model..."
            )
          }
        }

Comment on lines +65 to +75
} catch {
if error.isMLAssetUnavailable {
print(
"Local ML assets unavailable for Vision ID. Falling back to cloud model..."
)
} else {
print(
"Local model failed for Vision ID: \(error.localizedDescription). Falling back to cloud model..."
)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If the local model task is cancelled, the respond call will throw a cancellation error. Catching this error and proceeding directly to the cloud fallback is incorrect, as it will trigger an unwanted cloud API request for a cancelled operation. Guarding against cancellation at the start of the catch block prevents this resource waste.

        } catch {
          guard !Task.isCancelled else { return }
          if error.isMLAssetUnavailable {
            print(
              "Local ML assets unavailable for Vision ID. Falling back to cloud model..."
            )
          } else {
            print(
              "Local model failed for Vision ID: \(error.localizedDescription). Falling back to cloud model..."
            )
          }
        }

Comment on lines +62 to +66
.sheet(isPresented: $presentErrorDetails) {
if let error = viewModel.error {
ErrorDetailsView(error: error)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When the error sheet is dismissed, viewModel.error is not cleared. This leaves stale error state in the view model. Clearing the error on dismiss ensures that the view model state stays in sync with the UI presentation.

Suggested change
.sheet(isPresented: $presentErrorDetails) {
if let error = viewModel.error {
ErrorDetailsView(error: error)
}
}
.sheet(isPresented: $presentErrorDetails, onDismiss: { viewModel.error = nil }) {
if let error = viewModel.error {
ErrorDetailsView(error: error)
}
}

Comment on lines +27 to +31
TextEditor(text: $viewModel.inputText)
.frame(height: 120)
.padding(4)
.background(Color(.secondarySystemGroupedBackground))
.cornerRadius(8)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In SwiftUI, TextEditor has a default background color that covers any custom background modifier. To ensure that the .secondarySystemGroupedBackground color is visible behind the text editor, you should hide the default scroll content background using .scrollContentBackground(.hidden).

        TextEditor(text: $viewModel.inputText)
          .frame(height: 120)
          .scrollContentBackground(.hidden)
          .padding(4)
          .background(Color(.secondarySystemGroupedBackground))
          .cornerRadius(8)

@peterfriese peterfriese force-pushed the peterfriese/firebase-ai/fix-ci-build branch from 2e3f9e4 to 037d229 Compare July 12, 2026 04:54
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