Skip to content

fix(threads): remove runaway nThreads migration and show resolved Au… #313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dishit-wednesday merged 4 commits into from
Apr 21, 2026
Merged

fix(threads): remove runaway nThreads migration and show resolved Au… #313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
366b61e
fix(threads): remove runaway nThreads migration and show resolved Au…
dishit-wednesday Apr 20, 2026
e2622e9
fix: sync cpuThreads label fallback with slider value
dishit-wednesday Apr 21, 2026
c828ca4
fix test
dishit-wednesday Apr 21, 2026
101b4f0
fix test
dishit-wednesday Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions __tests__/rntl/screens/ModelSettingsScreen.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,10 @@
// Performance Settings
// ============================================================================
describe('performance settings', () => {
it('shows CPU Threads slider label and auto value when nThreads uses the auto sentinel', () => {
const { getByText } = renderWithSections('text');
it('shows CPU Threads slider label and auto value when nThreads uses the auto sentinel', async () => {
const { getByText, findByText } = renderWithSections('text');
expect(getByText('CPU Threads')).toBeTruthy();
expect(getByText('Auto')).toBeTruthy();
await findByText(/^Auto \(\d+\)$/);
});

it('shows Batch Size slider label and default value', () => {
Expand Down Expand Up @@ -832,12 +832,12 @@
},
});

const { getByText } = renderWithSections('image', 'text');
const { getByText, getAllByText } = renderWithSections('image', 'text');
// Verify fallback values are used
expect(getByText('0.70')).toBeTruthy(); // temperature || 0.7
expect(getByText('0.90')).toBeTruthy(); // topP || 0.9
expect(getByText('1.10')).toBeTruthy(); // repeatPenalty || 1.1
expect(getByText('6')).toBeTruthy(); // undefined still falls back to 6
expect(getAllByText('1').length).toBeGreaterThan(0); // undefined falls back to cpuThreadsSliderValue (1)
expect(getByText('8')).toBeTruthy(); // imageSteps || 8
expect(getByText('7.5')).toBeTruthy(); // imageGuidanceScale || 7.5
});
Expand Down Expand Up @@ -913,7 +913,7 @@
});

// HTP is currently disabled via HTP_UI_ENABLED feature flag
it.skip('locks KV cache display to f16 on HTP backend', () => {

Check warning on line 916 in __tests__/rntl/screens/ModelSettingsScreen.test.tsx

View workflow job for this annotation

GitHub Actions / lint 

Tests should not be skipped
useAppStore.getState().updateSettings({ inferenceBackend: 'htp', cacheType: 'q4_0' });
const { getByText } = renderWithSections('text');
expect(getByText(/Full precision/)).toBeTruthy();
Expand Down
6 changes: 4 additions & 2 deletions __tests__/unit/hooks/useTextGenerationAdvanced.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
});

// HTP is currently disabled via HTP_ENABLED feature flag
it.skip('locks KV cache to f16 when HTP backend is selected', () => {

Check warning on line 12 in __tests__/unit/hooks/useTextGenerationAdvanced.test.ts

View workflow job for this annotation

GitHub Actions / lint 

Tests should not be skipped
act(() => {
useAppStore.getState().updateSettings({ inferenceBackend: 'htp', cacheType: 'q4_0' });
});
Expand All @@ -21,14 +21,16 @@
expect(result.current.displayCacheType).toBe('f16');
});

it('shows Auto for cpu threads when nThreads uses the auto sentinel', () => {
it('shows Auto (N) for cpu threads when nThreads uses the auto sentinel', async () => {
act(() => {
useAppStore.getState().updateSettings({ nThreads: 0 });
});

const { result } = renderHook(() => useTextGenerationAdvanced());

expect(result.current.cpuThreadsDisplayValue).toBe('Auto');
await act(async () => {});

expect(result.current.cpuThreadsDisplayValue).toMatch(/^Auto \(\d+\)$/);
expect(result.current.cpuThreadsSliderValue).toBe(1);
});
});
13 changes: 12 additions & 1 deletion src/hooks/useTextGenerationAdvanced.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { useState, useEffect } from 'react';
import { Platform } from 'react-native';
import { useAppStore } from '../stores';
import { CacheType, INFERENCE_BACKENDS } from '../types';
import { hardwareService } from '../services/hardware';

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

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

To avoid a visible flicker from "Auto" to "Auto (N)" on every mount of the settings screen, and to prevent redundant asynchronous calls to fetch the hardware core count (which is static for the device), consider caching the resolved thread count in a module-level variable.

Suggested change
let cachedRecommendedThreads: number | null = null;

/** Feature flag: Set to true to enable HTP/Hexagon NPU support. Currently disabled. */
const HTP_ENABLED = false;
Expand Down Expand Up @@ -31,8 +33,17 @@
// OpenCL and HTP force f16 in the native loader, so lock the UI to match.
const cacheDisabled = gpuForcesF16;
const displayCacheType = cacheDisabled ? 'f16' : currentCacheType;
const [resolvedThreadCount, setResolvedThreadCount] = useState<number | null>(null);

useEffect(() => {
if (settings?.nThreads !== 0) return;
hardwareService.getRecommendedThreadCount().then(setResolvedThreadCount);
}, [settings?.nThreads]);
Comment on lines +36 to +41

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

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

Initialize the state with the cached value and update the cache once the recommended thread count is resolved. To prevent state updates on unmounted components or race conditions from stale renders, use a cancellation flag within the useEffect as per repository guidelines. The cache update itself can remain 'fire-and-forget' to ensure the value is available for future mounts even if the current one is cancelled.

  const [resolvedThreadCount, setResolvedThreadCount] = useState<number | null>(cachedRecommendedThreads);

  useEffect(() => {
    let isCancelled = false;
    if (settings?.nThreads !== 0 || resolvedThreadCount !== null) return;
    hardwareService.getRecommendedThreadCount().then(count => {
      cachedRecommendedThreads = count;
      if (isCancelled) return;
      setResolvedThreadCount(count);
    });
    return () => {
      isCancelled = true;
    };
  }, [settings?.nThreads, resolvedThreadCount]);
References
  1. When handling asynchronous operations in useEffect that update state, ensure to use a cancellation flag to prevent state updates on unmounted components.
  2. A 'fire-and-forget' asynchronous call is an acceptable pattern for a best-effort fallback to update a cache.


const cpuThreadsSliderValue = settings?.nThreads && settings.nThreads > 0 ? settings.nThreads : 1;
const cpuThreadsDisplayValue = settings?.nThreads === 0 ? 'Auto' : String(settings?.nThreads ?? 6);
const cpuThreadsDisplayValue = settings?.nThreads === 0
? (resolvedThreadCount != null ? `Auto (${resolvedThreadCount})` : 'Auto')

Check warning on line 45 in src/hooks/useTextGenerationAdvanced.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Unexpected negated condition. 


See more on https://sonarcloud.io/project/issues?id=alichherawalla_off-grid-mobile&issues=AZ2vNsp3LTotRVNfHy0x&open=AZ2vNsp3LTotRVNfHy0x&pullRequest=313

Check warning on line 45 in src/hooks/useTextGenerationAdvanced.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Extract this nested ternary operation into an independent statement. 


See more on https://sonarcloud.io/project/issues?id=alichherawalla_off-grid-mobile&issues=AZ2vNsp4LTotRVNfHy0y&open=AZ2vNsp4LTotRVNfHy0y&pullRequest=313
: String(cpuThreadsSliderValue);

const handleFlashAttnToggle = (flashAttn: boolean) => {
if (!flashAttn && isQuantizedCache) {
Expand Down
5 changes: 0 additions & 5 deletions src/stores/appStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,6 @@ function migratePersistedState(persistedState: any, currentState: AppState): App
if (merged.checklistDismissed && merged.onboardingChecklist &&
!Object.values(merged.onboardingChecklist).every(Boolean)) merged.checklistDismissed = false;
migrateEnabledTools(merged);
// nThreads: 4 was the old hard-coded default (before the 0 = auto sentinel).
// Migrate it to 0 so users get hardware-appropriate thread counts on next load.
if (merged.settings?.nThreads === 4) {
merged.settings = { ...merged.settings, nThreads: 0 };
}
return merged as AppState;
}

Expand Down
Loading