Skip to content

Commit 8bf0967

Browse files
Merge pull request #6 from alichherawalla/fix/model-download-storage-bugs
Fix/model download storage bugs
2 parents 2e8d4d7 + 60d5cef commit 8bf0967

5 files changed

Lines changed: 172 additions & 45 deletions

File tree

__tests__/unit/services/modelManager.test.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -495,10 +495,11 @@ describe('ModelManager', () => {
495495
describe('getOrphanedFiles', () => {
496496
it('finds untracked GGUF files', async () => {
497497
mockedRNFS.exists.mockResolvedValue(true);
498-
mockedRNFS.readDir.mockResolvedValue([
499-
{ name: 'orphan.gguf', path: '/models/orphan.gguf', size: 5000, isFile: () => true, isDirectory: () => false } as any,
500-
]);
501-
// getDownloadedModels returns empty (no tracked models)
498+
mockedRNFS.readDir
499+
.mockResolvedValueOnce([
500+
{ name: 'orphan.gguf', path: '/models/orphan.gguf', size: 5000, isFile: () => true, isDirectory: () => false } as any,
501+
])
502+
.mockResolvedValueOnce([]); // image models dir empty
502503
mockedAsyncStorage.getItem.mockResolvedValue('[]');
503504

504505
const orphaned = await modelManager.getOrphanedFiles();
@@ -509,9 +510,11 @@ describe('ModelManager', () => {
509510

510511
it('excludes tracked files', async () => {
511512
mockedRNFS.exists.mockResolvedValue(true);
512-
mockedRNFS.readDir.mockResolvedValue([
513-
{ name: 'tracked.gguf', path: '/models/tracked.gguf', size: 5000, isFile: () => true, isDirectory: () => false } as any,
514-
]);
513+
mockedRNFS.readDir
514+
.mockResolvedValueOnce([
515+
{ name: 'tracked.gguf', path: '/models/tracked.gguf', size: 5000, isFile: () => true, isDirectory: () => false } as any,
516+
])
517+
.mockResolvedValueOnce([]); // image models dir empty
515518
const storedModels = [{ id: 'm1', filePath: '/models/tracked.gguf', fileSize: 5000 }];
516519
mockedAsyncStorage.getItem.mockResolvedValue(JSON.stringify(storedModels));
517520

@@ -529,6 +532,25 @@ describe('ModelManager', () => {
529532

530533
expect(orphaned).toEqual([]);
531534
});
535+
536+
it('finds orphaned image model directories', async () => {
537+
mockedRNFS.exists.mockResolvedValue(true);
538+
mockedRNFS.readDir
539+
.mockResolvedValueOnce([]) // text models dir empty
540+
.mockResolvedValueOnce([
541+
{ name: 'anythingv5_cpu', path: '/image_models/anythingv5_cpu', size: 0, isFile: () => false, isDirectory: () => true } as any,
542+
])
543+
.mockResolvedValueOnce([ // contents of orphaned image model dir
544+
{ name: 'model.onnx', path: '/image_models/anythingv5_cpu/model.onnx', size: 500000, isFile: () => true, isDirectory: () => false } as any,
545+
]);
546+
mockedAsyncStorage.getItem.mockResolvedValue('[]');
547+
548+
const orphaned = await modelManager.getOrphanedFiles();
549+
550+
expect(orphaned).toHaveLength(1);
551+
expect(orphaned[0].name).toBe('anythingv5_cpu');
552+
expect(orphaned[0].size).toBe(500000);
553+
});
532554
});
533555

534556
// ========================================================================

android/app/src/main/java/com/localllm/download/DownloadManagerModule.kt

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,14 @@ class DownloadManagerModule(reactContext: ReactApplicationContext) :
111111
fun cancelDownload(downloadId: Double, promise: Promise) {
112112
try {
113113
val id = downloadId.toLong()
114+
115+
// Get download info BEFORE removing from SharedPreferences
116+
val downloadInfo = getDownloadInfo(id)
117+
114118
downloadManager.remove(id)
115119
removeDownload(id)
116120

117121
// Clean up partial file
118-
val downloadInfo = getDownloadInfo(id)
119122
downloadInfo?.optString("fileName")?.let { fileName ->
120123
val file = File(
121124
reactApplicationContext.getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS),
@@ -339,7 +342,8 @@ class DownloadManagerModule(reactContext: ReactApplicationContext) :
339342
android.util.Log.w("DownloadManager", "Download $downloadId has unknown status - may have completed or been removed")
340343
// Query the file directly to see if it completed
341344
val downloadInfo = getDownloadInfo(downloadId)
342-
downloadInfo?.optString("fileName")?.let { fileName ->
345+
val fileName = downloadInfo?.optString("fileName")
346+
if (fileName != null) {
343347
val file = java.io.File(
344348
reactApplicationContext.getExternalFilesDir(android.os.Environment.DIRECTORY_DOWNLOADS),
345349
fileName
@@ -351,7 +355,15 @@ class DownloadManagerModule(reactContext: ReactApplicationContext) :
351355
sendEvent("DownloadComplete", eventParams)
352356
}
353357
updateDownloadStatus(downloadId, "completed", file.toURI().toString())
358+
} else {
359+
// No file and no native download - stale entry, remove it
360+
android.util.Log.d("DownloadManager", "No file found for unknown download $downloadId, removing stale entry")
361+
removeDownload(downloadId)
354362
}
363+
} else {
364+
// No download info at all - remove stale entry
365+
android.util.Log.d("DownloadManager", "No info for unknown download $downloadId, removing stale entry")
366+
removeDownload(downloadId)
355367
}
356368
}
357369
}

src/screens/DownloadManagerScreen.tsx

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useEffect, useState, useCallback } from 'react';
1+
import React, { useEffect, useState, useCallback, useRef } from 'react';
22
import {
33
View,
44
Text,
@@ -39,6 +39,7 @@ export const DownloadManagerScreen: React.FC = () => {
3939
const [isRefreshing, setIsRefreshing] = useState(false);
4040
const [activeDownloads, setActiveDownloads] = useState<BackgroundDownloadInfo[]>([]);
4141
const [alertState, setAlertState] = useState<AlertState>(initialAlertState);
42+
const cancelledKeysRef = useRef<Set<string>>(new Set());
4243

4344
const {
4445
downloadedModels,
@@ -72,7 +73,9 @@ export const DownloadManagerScreen: React.FC = () => {
7273
if (Platform.OS !== 'android') return;
7374

7475
const unsubProgress = backgroundDownloadService.onAnyProgress((event) => {
75-
setDownloadProgress(`${event.modelId}/${event.fileName}`, {
76+
const key = `${event.modelId}/${event.fileName}`;
77+
if (cancelledKeysRef.current.has(key)) return;
78+
setDownloadProgress(key, {
7679
progress: event.totalBytes > 0 ? event.bytesDownloaded / event.totalBytes : 0,
7780
bytesDownloaded: event.bytesDownloaded,
7881
totalBytes: event.totalBytes,
@@ -131,20 +134,37 @@ export const DownloadManagerScreen: React.FC = () => {
131134
onPress: async () => {
132135
setAlertState(hideAlert());
133136
try {
134-
// Clear from progress tracking immediately (optimistic update)
137+
// Mark as cancelled so polling events don't re-add it
135138
const key = `${item.modelId}/${item.fileName}`;
139+
cancelledKeysRef.current.add(key);
140+
141+
// Clear from progress tracking immediately (optimistic update)
136142
setDownloadProgress(key, null);
137143

138-
// If it has a downloadId, cancel it via native download manager
139-
if (item.downloadId) {
140-
setBackgroundDownload(item.downloadId, null);
141-
await modelManager.cancelBackgroundDownload(item.downloadId);
144+
// Find downloadId - either from the item or by cross-referencing active downloads
145+
let downloadId = item.downloadId;
146+
if (!downloadId) {
147+
const match = activeDownloads.find(d => {
148+
const meta = activeBackgroundDownloads[d.downloadId];
149+
return meta && meta.fileName === item.fileName;
150+
});
151+
if (match) downloadId = match.downloadId;
152+
}
153+
154+
// Remove from local activeDownloads state immediately
155+
if (downloadId) {
156+
setActiveDownloads(prev => prev.filter(d => d.downloadId !== downloadId));
157+
setBackgroundDownload(downloadId, null);
158+
await modelManager.cancelBackgroundDownload(downloadId);
142159
}
143160

144161
// Wait a bit for native cancellation to complete, then reload
145-
// This prevents the polling from re-adding it before cancellation finishes
146162
setTimeout(async () => {
147163
await loadActiveDownloads();
164+
// Only clear cancelled key if native cancel succeeded
165+
if (downloadId) {
166+
cancelledKeysRef.current.delete(key);
167+
}
148168
}, 1000);
149169
} catch (error) {
150170
setAlertState(showAlert('Error', 'Failed to remove download'));

src/screens/StorageSettingsScreen.tsx

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ export const StorageSettingsScreen: React.FC = () => {
3131
const [isDeleting, setIsDeleting] = useState<string | null>(null);
3232
const [alertState, setAlertState] = useState<AlertState>(initialAlertState);
3333

34-
const { downloadedModels, activeBackgroundDownloads, setBackgroundDownload, clearBackgroundDownloads } = useAppStore();
34+
const { downloadedModels, downloadedImageModels, activeBackgroundDownloads, setBackgroundDownload, clearBackgroundDownloads } = useAppStore();
3535
const { conversations } = useChatStore();
3636

37+
const imageStorageUsed = downloadedImageModels.reduce((total, m) => total + (m.size || 0), 0);
38+
3739
// Find stale download entries (entries with missing/invalid data)
3840
const staleDownloads = Object.entries(activeBackgroundDownloads).filter(([_, info]) => {
3941
return !info || !info.modelId || !info.fileName || !info.totalBytes;
@@ -42,12 +44,12 @@ export const StorageSettingsScreen: React.FC = () => {
4244
useEffect(() => {
4345
loadStorageInfo();
4446
scanForOrphanedFiles();
45-
}, [downloadedModels]);
47+
}, [downloadedModels, downloadedImageModels]);
4648

4749
const loadStorageInfo = async () => {
4850
const used = await modelManager.getStorageUsed();
4951
const available = await modelManager.getAvailableStorage();
50-
setStorageUsed(used);
52+
setStorageUsed(used + imageStorageUsed);
5153
setAvailableStorage(available);
5254
};
5355

@@ -186,11 +188,19 @@ export const StorageSettingsScreen: React.FC = () => {
186188
<View style={styles.infoRow}>
187189
<View style={styles.infoRowLeft}>
188190
<Icon name="cpu" size={18} color={COLORS.primary} />
189-
<Text style={styles.infoLabel}>Downloaded Models</Text>
191+
<Text style={styles.infoLabel}>LLM Models</Text>
190192
</View>
191193
<Text style={styles.infoValue}>{downloadedModels.length}</Text>
192194
</View>
193195

196+
<View style={styles.infoRow}>
197+
<View style={styles.infoRowLeft}>
198+
<Icon name="image" size={18} color={COLORS.primary} />
199+
<Text style={styles.infoLabel}>Image Models</Text>
200+
</View>
201+
<Text style={styles.infoValue}>{downloadedImageModels.length}</Text>
202+
</View>
203+
194204
<View style={styles.infoRow}>
195205
<View style={styles.infoRowLeft}>
196206
<Icon name="hard-drive" size={18} color={COLORS.primary} />
@@ -210,7 +220,7 @@ export const StorageSettingsScreen: React.FC = () => {
210220

211221
{downloadedModels.length > 0 && (
212222
<Card style={styles.section}>
213-
<Text style={styles.sectionTitle}>Models</Text>
223+
<Text style={styles.sectionTitle}>LLM Models</Text>
214224
{downloadedModels.map((model, index) => (
215225
<View
216226
key={model.id}
@@ -229,6 +239,27 @@ export const StorageSettingsScreen: React.FC = () => {
229239
</Card>
230240
)}
231241

242+
{downloadedImageModels.length > 0 && (
243+
<Card style={styles.section}>
244+
<Text style={styles.sectionTitle}>Image Models</Text>
245+
{downloadedImageModels.map((model, index) => (
246+
<View
247+
key={model.id}
248+
style={[
249+
styles.modelRow,
250+
index === downloadedImageModels.length - 1 && styles.lastRow
251+
]}
252+
>
253+
<View style={styles.modelInfo}>
254+
<Text style={styles.modelName} numberOfLines={1}>{model.name}</Text>
255+
<Text style={styles.modelMeta}>{model.backend === 'qnn' ? 'Qualcomm NPU' : 'CPU'}{model.style ? ` • ${model.style}` : ''}</Text>
256+
</View>
257+
<Text style={styles.modelSize}>{hardwareService.formatBytes(model.size)}</Text>
258+
</View>
259+
))}
260+
</Card>
261+
)}
262+
232263
{/* Stale Downloads Section */}
233264
{staleDownloads.length > 0 && (
234265
<Card style={styles.section}>
@@ -287,8 +318,8 @@ export const StorageSettingsScreen: React.FC = () => {
287318
) : (
288319
<>
289320
<Text style={styles.warningText}>
290-
These GGUF files exist on disk but aren't tracked as models.
291-
They may be from failed downloads or manual copies.
321+
These files/folders exist on disk but aren't tracked as models.
322+
They may be from failed or cancelled downloads.
292323
</Text>
293324
{orphanedFiles.map((file) => (
294325
<View key={file.path} style={styles.orphanedRow}>

src/services/modelManager.ts

Lines changed: 63 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -297,33 +297,33 @@ class ModelManager {
297297
}
298298

299299
/**
300-
* Find GGUF files on disk that aren't tracked in the model list.
301-
* Returns array of orphaned file info.
300+
* Find files/directories on disk that aren't tracked in model registries.
301+
* Scans both the text models directory (for untracked files) and
302+
* the image models directory (for untracked directories).
302303
*/
303304
async getOrphanedFiles(): Promise<Array<{ name: string; path: string; size: number }>> {
304305
await this.initialize();
305306
const orphaned: Array<{ name: string; path: string; size: number }> = [];
306307

307308
try {
308-
const dirExists = await RNFS.exists(this.modelsDir);
309-
if (!dirExists) return orphaned;
310-
311-
const files = await RNFS.readDir(this.modelsDir);
312-
const models = await this.getDownloadedModels();
309+
// Scan text models directory for untracked files
310+
const modelsDirExists = await RNFS.exists(this.modelsDir);
311+
if (modelsDirExists) {
312+
const files = await RNFS.readDir(this.modelsDir);
313+
const models = await this.getDownloadedModels();
313314

314-
// Get all tracked file paths (including mmproj)
315-
const trackedPaths = new Set<string>();
316-
for (const model of models) {
317-
trackedPaths.add(model.filePath);
318-
if (model.mmProjPath) {
319-
trackedPaths.add(model.mmProjPath);
315+
// Get all tracked file paths (including mmproj)
316+
const trackedPaths = new Set<string>();
317+
for (const model of models) {
318+
trackedPaths.add(model.filePath);
319+
if (model.mmProjPath) {
320+
trackedPaths.add(model.mmProjPath);
321+
}
320322
}
321-
}
322323

323-
// Find GGUF files not in tracked list
324-
for (const file of files) {
325-
if (file.isFile() && file.name.endsWith('.gguf')) {
326-
if (!trackedPaths.has(file.path)) {
324+
// Find any files not in tracked list (not just .gguf)
325+
for (const file of files) {
326+
if (file.isFile() && !trackedPaths.has(file.path)) {
327327
orphaned.push({
328328
name: file.name,
329329
path: file.path,
@@ -332,6 +332,39 @@ class ModelManager {
332332
}
333333
}
334334
}
335+
336+
// Scan image models directory for untracked directories
337+
const imageDirExists = await RNFS.exists(this.imageModelsDir);
338+
if (imageDirExists) {
339+
const items = await RNFS.readDir(this.imageModelsDir);
340+
const imageModels = await this.getDownloadedImageModels();
341+
const trackedImagePaths = new Set(imageModels.map(m => m.modelPath));
342+
343+
for (const item of items) {
344+
if (!trackedImagePaths.has(item.path)) {
345+
let totalSize = 0;
346+
if (item.isDirectory()) {
347+
try {
348+
const dirFiles = await RNFS.readDir(item.path);
349+
for (const f of dirFiles) {
350+
if (f.isFile()) {
351+
totalSize += typeof f.size === 'string' ? parseInt(f.size, 10) : f.size;
352+
}
353+
}
354+
} catch {
355+
// Can't read directory, use 0
356+
}
357+
} else {
358+
totalSize = typeof item.size === 'string' ? parseInt(item.size, 10) : item.size;
359+
}
360+
orphaned.push({
361+
name: item.name,
362+
path: item.path,
363+
size: totalSize,
364+
});
365+
}
366+
}
367+
}
335368
} catch (error) {
336369
console.error('[ModelManager] Error scanning for orphaned files:', error);
337370
}
@@ -340,12 +373,15 @@ class ModelManager {
340373
}
341374

342375
/**
343-
* Delete an orphaned file from disk.
376+
* Delete an orphaned file or directory from disk.
344377
*/
345378
async deleteOrphanedFile(filePath: string): Promise<void> {
346379
try {
347-
await RNFS.unlink(filePath);
348-
console.log('[ModelManager] Deleted orphaned file:', filePath);
380+
const exists = await RNFS.exists(filePath);
381+
if (exists) {
382+
await RNFS.unlink(filePath);
383+
console.log('[ModelManager] Deleted orphaned file/directory:', filePath);
384+
}
349385
} catch (error) {
350386
console.error('[ModelManager] Failed to delete orphaned file:', error);
351387
throw error;
@@ -1080,6 +1116,12 @@ class ModelManager {
10801116

10811117
const fileSize = typeof item.size === 'string' ? parseInt(item.size, 10) : item.size;
10821118

1119+
// Skip tiny files (< 1MB) — likely partial/failed downloads, not valid models
1120+
if (fileSize < 1_000_000) {
1121+
console.log(`[ModelManager] Skipping tiny file as likely partial download: ${item.name} (${fileSize} bytes)`);
1122+
continue;
1123+
}
1124+
10831125
// Try to parse quantization from filename
10841126
const quantMatch = item.name.match(/[_-](Q\d+[_\w]*|f16|f32)/i);
10851127
const quantization = quantMatch ? quantMatch[1].toUpperCase() : 'Unknown';

0 commit comments

Comments
 (0)