Skip to content
Closed
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/**
* RED-FLOW (integration) — image-model download stuck "failed" after the completed bytes are lost.
*
* DEVICE GROUND TRUTH (iOS, production build): SDXL (Core ML) completed (2.8/2.8 GB) then showed
* "…coreml_apple_…xl-base-ios couldn't be opened because there is no such file", and Retry did
* nothing. Root cause (native, fixed separately): the completed file was staged in
* NSTemporaryDirectory(), which iOS purges across relaunch — so finalize (moveCompletedDownload →
* unzip) ran against a file the OS had deleted.
*
* This guards the shared JS half: when the completed bytes are unrecoverable (native
* moveCompletedDownload rejects "no such file" AND no valid zip/extracted dir survives on disk),
* the retry-finalize path must RE-DOWNLOAD from scratch instead of dead-ending. Before the fix,
* resumeZipDownload rethrew and the row stuck at 'failed' with no fresh download.
*
* resumeImageDownload is SHARED (it runs on iOS retry AND on Android app-open resume), so this runs
* on BOTH platforms with a device-faithful fixture each (iOS = CoreML zip; Android = MNN zip). The
* fallback is additive: on the normal Android path moveCompletedDownload succeeds and this branch
* never fires, so this only makes the FAILURE case recover instead of dead-end — on both platforms.
*
* Falsifier: revert the reDownloadFromMetadata fallback in imageDownloadResume → the entry stays
* 'failed' and no new native download row appears → RED (verified on both platforms).
*/
import { installNativeBoundary } from '../../harness/nativeBoundary';

const FIXTURES = {
ios: {
modelId: 'coreml_apple_coreml-stable-diffusion-xl-base-ios',
backend: 'coreml',
name: 'SDXL (iOS)',
downloadUrl: 'https://huggingface.co/apple/coreml-stable-diffusion-xl-base-ios/resolve/main/split_einsum/compiled.zip',
},
android: {
modelId: 'anythingv5-mnn',
backend: 'mnn',
name: 'Anything V5 (MNN)',
downloadUrl: 'https://example.com/models/anythingv5-mnn.zip',
},
} as const;

describe.each(['ios', 'android'] as const)(
'image staging purged — retry re-downloads instead of dead-ending, on %s (red-flow)',
(platform) => {
afterEach(() => {
const { Platform } = require('react-native');
Object.defineProperty(Platform, 'OS', { value: 'ios', configurable: true });
});

it('re-issues a fresh download when the completed bytes are gone at finalize', async () => {
const boundary = installNativeBoundary({ download: true, fs: true });
const { Platform } = require('react-native');
Object.defineProperty(Platform, 'OS', { value: platform, configurable: true });
const { useDownloadStore, isActiveStatus } = require('../../../src/stores/downloadStore');
const { useAppStore } = require('../../../src/stores');
const { makeImageModelKey } = require('../../../src/utils/modelKey');
const { resumeImageDownload } = require('../../../src/screens/ModelsScreen/imageDownloadResume');

const fx = FIXTURES[platform];
const fileName = `${fx.modelId}.zip`;
const modelKey = makeImageModelKey(fx.modelId);
const total = 2.8 * 1024 * 1024 * 1024;

// A completed image zip download that failed finalize: bytes fully present, marked 'failed'.
// metadata carries a real download URL (the zip download type) so a re-download is possible.
useDownloadStore.getState().add({
modelKey,
downloadId: 'dl-img',
modelId: `image:${fx.modelId}`,
fileName,
quantization: '',
modelType: 'image',
status: 'failed',
bytesDownloaded: total,
totalBytes: total,
combinedTotalBytes: total,
progress: 1,
createdAt: 1,
metadataJson: JSON.stringify({
imageDownloadType: 'zip',
imageModelName: fx.name,
imageModelDescription: 'test model',
imageModelSize: total,
imageModelStyle: 'realistic',
imageModelBackend: fx.backend,
imageModelAttentionVariant: 'split_einsum',
imageModelDownloadUrl: fx.downloadUrl,
}),
});

// BOUNDARY (the device fact this reproduces): the completed file was staged and then lost
// (iOS temp purge / storage clear), so the native move of the staged source now fails
// "no such file". Nothing valid survives on disk — the unrecoverable case.
boundary.download!.module.moveCompletedDownload.mockRejectedValue(
new Error(`The file "download_dl-img_${fileName}" couldn't be opened because there is no such file.`),
);

const entry = useDownloadStore.getState().downloads[modelKey];
const appState = useAppStore.getState();
const deps = {
addDownloadedImageModel: appState.addDownloadedImageModel,
activeImageModelId: appState.activeImageModelId,
setActiveImageModelId: appState.setActiveImageModelId,
setAlertState: () => {},
triedImageGen: false,
};

// Precondition: no native download in flight, and the row is a dead 'failed' (the screenshot state).
expect(boundary.download!.active().length).toBe(0);

Check warning on line 107 in __tests__/integration/downloads/iosImageStagingPurgedRedownloads.redflow.test.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer "expect(boundary.download!.active()).toHaveLength(0)" over this generic assertion for better reporting; it works on any object with a numeric length property.

See more on https://sonarcloud.io/project/issues?id=off-grid-ai_off-grid-ai-mobile&issues=AZ9lAKzV77db1dr_kiQv&open=AZ9lAKzV77db1dr_kiQv&pullRequest=557
expect(useDownloadStore.getState().downloads[modelKey].status).toBe('failed');

// ACT: the retry/resume-finalize path for an all-bytes-present image download.
await resumeImageDownload(entry, deps);

// Recovery: a FRESH download is now in flight (real native row) and the row is active again —
// not stuck 'failed'. RED before the fix: no new row, status stays 'failed'.
const rows = boundary.download!.active();
expect(rows.some(r => r.modelId === `image:${fx.modelId}` || r.fileName === fileName)).toBe(true);
expect(isActiveStatus(useDownloadStore.getState().downloads[modelKey].status)).toBe(true);
expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed');
});
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* RED-FLOW (UI, rendered) — the iOS image-download "stuck failed" bug at the pixel.
*
* DEVICE (production build): SDXL (Core ML) completed (2.8/2.8 GB) then showed a failed card with
* "…couldn't be opened because there is no such file", and tapping Retry did nothing. Root cause:
* the completed bytes were staged in NSTemporaryDirectory() (native, fixed separately) and lost;
* finalize + every retry re-ran moveCompletedDownload on the dead download → same error.
*
* This mounts the REAL DownloadManagerScreen over the download-native + FS fakes, taps the REAL
* Retry button a user taps, and asserts the card RECOVERS (no longer failed; a fresh download is in
* flight). Before the JS fix, resumeZipDownload rethrew and the row stayed failed → the Retry button
* is still there and no new download exists → RED. Fakes only at the native boundary; the real
* screen, provider, retry wiring, resume/finalize, store and proceedWithDownload all run.
*/
import { installNativeBoundary, requireRTL } from '../../harness/nativeBoundary';

describe('rendered — iOS image staging purged: Retry recovers the failed card', () => {
it('taps Retry on the failed SDXL card and the download recovers (not stuck failed)', async () => {
const boundary = installNativeBoundary({ download: true, fs: true });
const React = require('react');
// The device bug is iOS (Core ML, production build). Pin the platform so imageProvider.retry takes
// the iOS path (imageOps.retry → resume/finalize) and not the Android native-resume branch.
const { Platform } = require('react-native');
Object.defineProperty(Platform, 'OS', { value: 'ios', configurable: true });
const { render, waitFor, fireEvent } = requireRTL();
const { useDownloadStore } = require('../../../src/stores/downloadStore');
const { makeImageModelKey } = require('../../../src/utils/modelKey');
const { DownloadManagerScreen } = require('../../../src/screens/DownloadManagerScreen');
const { registerCoreDownloadProviders } = require('../../../src/services/modelDownloadService/registerProviders');
// The download service has no providers until the app registers them at startup — without this
// modelDownloadService.retry() finds no owning provider and silently refuses (status stays failed).
registerCoreDownloadProviders();

const modelId = 'coreml_apple_coreml-stable-diffusion-xl-base-ios';
const fileName = `${modelId}.zip`;
const modelKey = makeImageModelKey(modelId);
const total = 2.8 * 1024 * 1024 * 1024;

// A completed CoreML zip download that failed finalize: bytes fully present, status 'failed', no
// errorCode (so it renders as retryable — the failed card shows the Retry button).
useDownloadStore.getState().add({
modelKey, downloadId: 'dl-sdxl', modelId: `image:${modelId}`, fileName,
quantization: '', modelType: 'image', status: 'failed',
bytesDownloaded: total, totalBytes: total, combinedTotalBytes: total, progress: 1, createdAt: 1,
metadataJson: JSON.stringify({
imageDownloadType: 'zip', imageModelName: 'SDXL (iOS)', imageModelDescription: 'test',
imageModelSize: total, imageModelStyle: 'realistic', imageModelBackend: 'coreml',
imageModelAttentionVariant: 'split_einsum',
imageModelRepo: 'apple/coreml-stable-diffusion-xl-base-ios',
imageModelDownloadUrl: 'https://huggingface.co/apple/coreml-stable-diffusion-xl-base-ios/resolve/main/split_einsum/compiled.zip',
}),
});

// BOUNDARY: the staged completed file was purged, so the native move now fails "no such file",
// and nothing valid survives on disk — the unrecoverable case that trapped retry.
boundary.download!.module.moveCompletedDownload.mockRejectedValue(
new Error(`The file "download_dl-sdxl_${fileName}" couldn't be opened because there is no such file.`),
);

const view = render(React.createElement(DownloadManagerScreen, {}));

// Precondition: the failed SDXL card is on screen WITH a Retry button (the screenshot state).
const retry = await waitFor(() => {
const btn = view.queryByTestId('failed-retry-button');
expect(btn).not.toBeNull();
return btn;
});
expect(view.queryByText(/SDXL|coreml_apple/)).not.toBeNull();
expect(boundary.download!.active().length).toBe(0);

Check warning on line 69 in __tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer "expect(boundary.download!.active()).toHaveLength(0)" over this generic assertion for better reporting; it works on any object with a numeric length property.

See more on https://sonarcloud.io/project/issues?id=off-grid-ai_off-grid-ai-mobile&issues=AZ9lFgbqQHxy7zZzhO06&open=AZ9lFgbqQHxy7zZzhO06&pullRequest=557

// GESTURE: tap Retry, the way the user did on the device.
fireEvent.press(retry!);

// RECOVERY (what the user should now see): the Retry button is gone because the row is no longer
// failed — a fresh download is in flight. RED before the fix: still failed, Retry still there, no
// new download row.
await waitFor(() => {
expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed');
}, { timeout: 5000 });
expect(view.queryByTestId('failed-retry-button')).toBeNull();
const rows = boundary.download!.active();
expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true);
Comment on lines +77 to +82

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Move all recovery assertions inside waitFor to prevent test flakiness.

React state updates and renders can be batched and asynchronous. Waiting only for the Zustand store to update does not guarantee that React has finished committing the corresponding UI changes. Asserting the DOM synchronously immediately after the store update can lead to a race condition where the UI still reflects the old state.

Moving the UI and boundary assertions into the waitFor block ensures both the state and the DOM are fully updated and synchronized before proceeding. As per coding guidelines, integration tests should "assert what the user sees", and asserting the UI directly inside the asynchronous wait properly aligns with this principle.

💚 Proposed fix to group assertions
     await waitFor(() => {
       expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed');
-    }, { timeout: 5000 });
-    expect(view.queryByTestId('failed-retry-button')).toBeNull();
-    const rows = boundary.download!.active();
-    expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true);
+      expect(view.queryByTestId('failed-retry-button')).toBeNull();
+      const rows = boundary.download!.active();
+      expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true);
+    }, { timeout: 5000 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await waitFor(() => {
expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed');
}, { timeout: 5000 });
expect(view.queryByTestId('failed-retry-button')).toBeNull();
const rows = boundary.download!.active();
expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true);
await waitFor(() => {
expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed');
expect(view.queryByTestId('failed-retry-button')).toBeNull();
const rows = boundary.download!.active();
expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true);
}, { timeout: 5000 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@__tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsx`
around lines 77 - 82, Move the failed-retry-button and boundary active-row
assertions into the existing waitFor callback alongside the download status
assertion. Keep retrying until the store status, rendered UI, and boundary rows
all reflect successful recovery, while preserving the current assertion
conditions.

Source: Coding guidelines

});
});
35 changes: 31 additions & 4 deletions ios/DownloadManagerModule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,25 @@ class DownloadManagerModule: RCTEventEmitter {
return resolved.hasPrefix(documentsDir) || resolved.hasPrefix(cachesDir) || resolved.hasPrefix(tmpDir)
}

// MARK: - Durable staging

/// A DURABLE directory to stage a completed download until JS finalizes it (moves it to the model
/// dir / unzips it). This MUST NOT be NSTemporaryDirectory(): iOS reaps the temp dir across app
/// relaunches and under memory pressure, so a completed-but-not-yet-finalized file staged there is
/// gone by the time finalization runs — especially after the queued-survival rework, which now
/// restores and finalizes downloads AFTER a relaunch. A large image zip (multi-GB) is the worst case:
/// it commonly spans a backgrounding/relaunch before its unzip completes, and the vanished staged
/// zip surfaced as "…couldn't be opened because there is no such file" on iOS. Documents is durable
/// (never auto-purged), inside the sandbox allowlist, and we exclude it from iCloud backup.
static func completedStagingDirectory() -> String {
let documentsDir = NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true).first
?? NSTemporaryDirectory()
let dir = (documentsDir as NSString).appendingPathComponent(".download-staging")
try? FileManager.default.createDirectory(atPath: dir, withIntermediateDirectories: true)
excludeFromBackup(at: URL(fileURLWithPath: dir))
return dir
}

// MARK: - RCTEventEmitter

override init() {
Expand Down Expand Up @@ -1178,8 +1197,10 @@ extension DownloadManagerModule {
downloadId: String,
info: inout DownloadInfo,
fileManager: FileManager) {
let tmpDir = NSTemporaryDirectory()
let destPath = "\(tmpDir)/download_\(downloadId)_\(info.fileName)"
// Stage the completed file in a DURABLE dir (not NSTemporaryDirectory, which iOS purges across
// relaunch / under memory pressure) so a finalize that runs after a relaunch can still find it.
let stagingDir = DownloadManagerModule.completedStagingDirectory()
let destPath = "\(stagingDir)/download_\(downloadId)_\(info.fileName)"
let destURL = URL(fileURLWithPath: destPath)
try? fileManager.removeItem(at: destURL)

Expand All @@ -1188,6 +1209,9 @@ extension DownloadManagerModule {
do {
try fileManager.moveItem(at: location, to: destURL)
NSLog("[DownloadManager] Single file saved to: %@", destPath)
// Exclude the staged file itself from backup (per-file flag; the dir flag is not inherited by
// files created later). These are large model artifacts we never want in an iCloud backup.
DownloadManagerModule.excludeFromBackup(at: destURL)
info.localUri = destPath
info.status = "completed"
info.bytesDownloaded = info.totalBytes
Expand Down Expand Up @@ -1305,9 +1329,12 @@ class DownloadSessionDelegate: NSObject, URLSessionDownloadDelegate {
downloadTask.taskIdentifier, location.path)

// CRITICAL: The file at `location` is deleted by URLSession as soon as this method returns.
// We must copy it to a safe location SYNCHRONOUSLY before returning.
// We must copy it to a safe location SYNCHRONOUSLY before returning. That location must be
// DURABLE (not NSTemporaryDirectory) — if the app is killed between here and handleCompletion,
// a temp copy would be reaped and the completed bytes lost. See completedStagingDirectory().
let fileManager = FileManager.default
let safeTmp = NSTemporaryDirectory() + "dl_task_\(downloadTask.taskIdentifier)_\(UUID().uuidString).tmp"
let stagingDir = DownloadManagerModule.completedStagingDirectory()
let safeTmp = "\(stagingDir)/dl_task_\(downloadTask.taskIdentifier)_\(UUID().uuidString).tmp"
Comment on lines +1332 to +1337

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Resource leak: .tmp files are not cleaned up on failure or app termination.

Because .download-staging is a durable directory (unlike NSTemporaryDirectory), any .tmp files left here will persist indefinitely. These are multi-GB model artifacts, so leaking them will quickly exhaust device storage.

Currently, safeTmp files leak in three scenarios:

  1. Single-file error: If moveItem fails in handleSingleFileCompletion, location (the .tmp file) is left behind.
  2. Multi-file fallback: If moveItem fails in handleMultiFileTaskCompletion and falls back to copyItem, location is left behind regardless of whether the copy succeeds or fails.
  3. App termination: If the app is killed between didFinishDownloadingTo and the async handleCompletion block, the .tmp file is orphaned. Since URLSession foreground tasks do not survive restart, the file will never be processed.

To fix this, clean up location in the error/fallback paths of the completion handlers, and clear any orphaned .tmp files when initializing the session.

♻️ Proposed fixes to prevent `.tmp` file leaks

1. Clean up in handleSingleFileCompletion (around line 1238):

     } catch {
       NSLog("[DownloadManager] Failed to move single file: %@", error.localizedDescription)
+      try? fileManager.removeItem(at: location)
       info.status = "failed"
       taskToDownloadId.removeValue(forKey: taskId)

2. Clean up in handleMultiFileTaskCompletion (around line 1151):

       } catch {
         NSLog("[DownloadManager] Failed to save file %@: %@",
               fileTask.relativePath, error.localizedDescription)
       }
+      try? fileManager.removeItem(at: location)
     }

     fileTask.completed = true

3. Clean up orphaned .tmp files on startup (add inside or immediately after setupSession()):

queue.async(flags: .barrier) {
  let stagingDir = DownloadManagerModule.completedStagingDirectory()
  if let files = try? FileManager.default.contentsOfDirectory(atPath: stagingDir) {
    for file in files where file.hasSuffix(".tmp") {
      try? FileManager.default.removeItem(atPath: "\(stagingDir)/\(file)")
    }
  }
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ios/DownloadManagerModule.swift` around lines 1332 - 1337, Clean up staged
.tmp files in the completion and startup flows: update
handleSingleFileCompletion and handleMultiFileTaskCompletion to remove location
when moving fails or before/after the multi-file copy fallback, and initialize
cleanup in or immediately after setupSession() to delete orphaned .tmp files
from completedStagingDirectory().

let safeURL = URL(fileURLWithPath: safeTmp)

do {
Expand Down
8 changes: 8 additions & 0 deletions scripts/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ gh release create "v${NEW_VERSION}" \
--title "Off Grid v${NEW_VERSION}" \
--notes-file "$NOTES_FILE"

# Announce the release in Slack — fail-soft, a chat message must never fail a published release.
# Webhook comes from .env.keygen locally (mirrors the SLACK_WEBHOOK_URL repo secret the CI release
# workflows use); notify-slack-release.mjs no-ops if it is unset.
SLACK_WEBHOOK_URL="${SLACK_WEBHOOK_URL:-$(sed -n 's/^SLACK_WEBHOOK_URL=//p' "$ROOT_DIR/.env.keygen" 2>/dev/null)}" \
PRODUCT="Off Grid AI Mobile" VERSION="$NEW_VERSION" CHANNEL_LABEL="stable" \
RELEASE_URL="$(gh release view "v${NEW_VERSION}" --json url -q .url 2>/dev/null)" NOTES_FILE="$NOTES_FILE" \
node "$ROOT_DIR/scripts/notify-slack-release.mjs" || true

# Clean up temp file
rm -f "$NOTES_FILE"

Expand Down
8 changes: 8 additions & 0 deletions scripts/uat.sh
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,14 @@ fi
printf '\n\n%s\n' "$BUILD_LINE" >> "$NOTES_FILE"
gh release create "$TAG" "${GH_ARGS[@]}" --prerelease --title "Off Grid ${BETA_VERSION} (beta)" --notes-file "$NOTES_FILE"

# Announce the beta in Slack — fail-soft, a chat message must never fail a shipped build. Webhook comes
# from .env.keygen locally (mirrors the SLACK_WEBHOOK_URL repo secret the release workflows use);
# notify-slack-release.mjs no-ops if it is unset.
SLACK_WEBHOOK_URL="${SLACK_WEBHOOK_URL:-$(sed -n 's/^SLACK_WEBHOOK_URL=//p' "$ROOT_DIR/.env.keygen" 2>/dev/null)}" \
PRODUCT="Off Grid AI Mobile" VERSION="$BETA_VERSION" CHANNEL_LABEL="beta" \
RELEASE_URL="$(gh release view "$TAG" --json url -q .url 2>/dev/null)" NOTES_FILE="$NOTES_FILE" \
node "$ROOT_DIR/scripts/notify-slack-release.mjs" || true

rm -f "$NOTES_FILE" "${ANDROID_CHANGELOG:-}" "$APK_DST" "$AAB_DST"
echo ""
info "${BOLD}Beta ${BETA_VERSION} shipped.${NC}"
Expand Down
16 changes: 2 additions & 14 deletions src/screens/DownloadManagerScreen/retryHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { backgroundDownloadService } from '../../services';
import { DownloadItem } from './items';
import logger from '../../utils/logger';
import { proceedWithDownload } from '../ModelsScreen/imageDownloadActions';
import { imageDescriptorFromMetadata } from '../ModelsScreen/imageDescriptor';
import { resumeImageDownload } from '../ModelsScreen/imageDownloadResume';

export function parseEntryMetadata(entry: DownloadEntry): Record<string, any> | null {
Expand Down Expand Up @@ -60,20 +61,7 @@ async function retryIosImageDownload(entry: DownloadEntry, setAlertState: (s: Al
setAlertState,
triedImageGen: appState.onboardingChecklist.triedImageGen,
};
await proceedWithDownload({
id: modelId,
name: meta.imageModelName,
description: meta.imageModelDescription,
downloadUrl: meta.imageModelDownloadUrl ?? '',
size: meta.imageModelSize,
style: meta.imageModelStyle,
backend: meta.imageModelBackend,
attentionVariant: meta.imageModelAttentionVariant,
huggingFaceRepo: meta.imageModelRepo,
huggingFaceFiles: meta.imageModelHuggingFaceFiles,
coremlFiles: meta.imageModelCoremlFiles,
repo: meta.imageModelRepo,
}, deps);
await proceedWithDownload(imageDescriptorFromMetadata(modelId, meta), deps);
}

/**
Expand Down
23 changes: 23 additions & 0 deletions src/screens/ModelsScreen/imageDescriptor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { ImageModelDescriptor } from './types';

/** Reconstruct an ImageModelDescriptor from a download entry's persisted metadata — the SINGLE
* source for "re-download this image model from what we remembered about it". Used by the iOS
* retry path (retryHandlers) and by resume's re-download-on-unrecoverable fallback so the two
* can't drift. Pure (zero-IO). Safe defaults keep it valid; undefined coreml/hf fields route it
* to the zip download path. */
export function imageDescriptorFromMetadata(modelId: string, meta: Record<string, any>): ImageModelDescriptor {
return {
id: modelId,
name: meta.imageModelName,
description: meta.imageModelDescription ?? '',
downloadUrl: meta.imageModelDownloadUrl ?? '',
size: meta.imageModelSize ?? 0,
style: meta.imageModelStyle ?? '',
backend: meta.imageModelBackend ?? 'coreml',
attentionVariant: meta.imageModelAttentionVariant,
huggingFaceRepo: meta.imageModelRepo,
huggingFaceFiles: meta.imageModelHuggingFaceFiles,
coremlFiles: meta.imageModelCoremlFiles,
repo: meta.imageModelRepo,
};
}
Comment on lines +8 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Provide a fallback for the required name property.

The name property is required by the ImageModelDescriptor interface, but meta.imageModelName is mapped without a default. If it is undefined in the persisted metadata, it could violate the downstream contract. Consider adding a fallback (e.g., ?? '') for consistency with the other string properties.

🐛 Proposed fix
-    name: meta.imageModelName,
+    name: meta.imageModelName ?? '',
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function imageDescriptorFromMetadata(modelId: string, meta: Record<string, any>): ImageModelDescriptor {
return {
id: modelId,
name: meta.imageModelName,
description: meta.imageModelDescription ?? '',
downloadUrl: meta.imageModelDownloadUrl ?? '',
size: meta.imageModelSize ?? 0,
style: meta.imageModelStyle ?? '',
backend: meta.imageModelBackend ?? 'coreml',
attentionVariant: meta.imageModelAttentionVariant,
huggingFaceRepo: meta.imageModelRepo,
huggingFaceFiles: meta.imageModelHuggingFaceFiles,
coremlFiles: meta.imageModelCoremlFiles,
repo: meta.imageModelRepo,
};
}
export function imageDescriptorFromMetadata(modelId: string, meta: Record<string, any>): ImageModelDescriptor {
return {
id: modelId,
name: meta.imageModelName ?? '',
description: meta.imageModelDescription ?? '',
downloadUrl: meta.imageModelDownloadUrl ?? '',
size: meta.imageModelSize ?? 0,
style: meta.imageModelStyle ?? '',
backend: meta.imageModelBackend ?? 'coreml',
attentionVariant: meta.imageModelAttentionVariant,
huggingFaceRepo: meta.imageModelRepo,
huggingFaceFiles: meta.imageModelHuggingFaceFiles,
coremlFiles: meta.imageModelCoremlFiles,
repo: meta.imageModelRepo,
};
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/screens/ModelsScreen/imageDescriptor.ts` around lines 8 - 23, Update
imageDescriptorFromMetadata so the required ImageModelDescriptor name field uses
an empty-string fallback when meta.imageModelName is nullish, matching the
existing handling of other string properties.

Loading
Loading