Skip to content

Commit e26ac83

Browse files
chore(sonar): clear all 27 SonarCloud PR issues
All on PR-changed code (leak period). No behaviour change — gated by tsc + full suite (7741). - S7763 (11): re-export patterns → `export … from` (useChatScreen, CustomAlert, downloadStore), keeping internally-used symbols as normal imports. - S3863 (2): merged the duplicate `./types` import in imageDownloadQnn. - S4782 (1): extracted a named `RemoteCaps` type in engines.ts (drops the indexed-access `undefined`). - S3776 (2 CRITICAL): reduced cognitive complexity — handleRetryMessageFn split into retryFromUserMessage/retryFromAssistantMessage helpers; parseThinkingContent split into parseGemmaThinking/parseChannelThinking/parseThinkTags (behavior-preserving, contract tests green). - S3358 (1): killed the nested ternary via assistantRetryKind() helper. - S6594 (7): `content.match(/re/)` → `/re/.exec(content)` (non-global, same semantics). - S7780 (3): String.raw for the regex-building template strings. - S5976 (1): parameterized the 3 platform-floor residency tests with it.each (real seam, real outcome). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f92631a commit e26ac83

8 files changed

Lines changed: 122 additions & 176 deletions

File tree

__tests__/unit/services/modelResidency.test.ts

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -794,43 +794,24 @@ describe('ModelResidencyManager', () => {
794794
const originalOS = RN.Platform.OS;
795795
afterEach(() => { RN.Platform.OS = originalOS; });
796796

797-
it('ANDROID: a ~3.7GB dirty model that leaves ~800MB physical LOADS (was refused by the iOS 1200 floor)', async () => {
798-
RN.Platform.OS = 'android';
797+
// All three share the same real seam (makeRoomFor with override on a dirty model, 4500MB
798+
// physical); only platform + model size differ, driving a different floor verdict. Physical
799+
// is always the guard — no swap credit — so an oversized dirty model is refused on both.
800+
it.each([
801+
{ os: 'android', sizeMB: 3700, expected: true, why: 'leaves 800MB > 700 android floor → LOADS (the 12GB-phone case)' },
802+
{ os: 'android', sizeMB: 5235, expected: false, why: '4500-5235 negative → refused (the E4B OOM guard, no swap credit)' },
803+
{ os: 'ios', sizeMB: 3700, expected: false, why: 'leaves 800MB < 1200 iOS jetsam floor → refused' },
804+
])('$os: a $sizeMB MB dirty model → fits=$expected ($why)', async ({ os, sizeMB, expected }) => {
805+
RN.Platform.OS = os;
799806
modelResidencyManager.setBudgetOverrideMB(null);
800807
jest.spyOn(hardwareService, 'refreshMemoryInfo').mockResolvedValue(undefined as never);
801808
jest.spyOn(hardwareService, 'getTotalMemoryGB').mockReturnValue(12);
802809
jest.spyOn(hardwareService, 'getAvailableMemoryGB').mockReturnValue(4.5); // ~4500MB physical
803810
const { fits } = await modelResidencyManager.makeRoomFor(
804-
{ key: 'text', type: 'text', sizeMB: 3700, dirtyMemory: true }, // 4500-3700=800 > 700 android floor
811+
{ key: 'text', type: 'text', sizeMB, dirtyMemory: true },
805812
{ override: true },
806813
);
807-
expect(fits).toBe(true); // loads on Android (800 > 700) — the user's 12GB-phone case
808-
});
809-
810-
it('ANDROID: an oversized dirty model that exceeds PHYSICAL is still refused (the OOM guard, no swap credit)', async () => {
811-
RN.Platform.OS = 'android';
812-
modelResidencyManager.setBudgetOverrideMB(null);
813-
jest.spyOn(hardwareService, 'refreshMemoryInfo').mockResolvedValue(undefined as never);
814-
jest.spyOn(hardwareService, 'getTotalMemoryGB').mockReturnValue(12);
815-
jest.spyOn(hardwareService, 'getAvailableMemoryGB').mockReturnValue(4.5);
816-
const { fits } = await modelResidencyManager.makeRoomFor(
817-
{ key: 'text', type: 'text', sizeMB: 5235, dirtyMemory: true }, // 4500-5235 = negative → refuse (the E4B OOM case)
818-
{ override: true },
819-
);
820-
expect(fits).toBe(false); // refused — a 5.2GB dirty model can't fit ~4.5GB physical (would OOM)
821-
});
822-
823-
it('iOS: the same ~3.7GB dirty model is REFUSED (full jetsam floor stands — no user swap)', async () => {
824-
RN.Platform.OS = 'ios';
825-
modelResidencyManager.setBudgetOverrideMB(null);
826-
jest.spyOn(hardwareService, 'refreshMemoryInfo').mockResolvedValue(undefined as never);
827-
jest.spyOn(hardwareService, 'getTotalMemoryGB').mockReturnValue(12);
828-
jest.spyOn(hardwareService, 'getAvailableMemoryGB').mockReturnValue(4.5);
829-
const { fits } = await modelResidencyManager.makeRoomFor(
830-
{ key: 'text', type: 'text', sizeMB: 3700, dirtyMemory: true }, // 4500-3700=800 < 1200 iOS floor
831-
{ override: true },
832-
);
833-
expect(fits).toBe(false); // iOS unchanged — 800 < 1200 jetsam floor
814+
expect(fits).toBe(expected);
834815
});
835816
});
836817
});

src/components/CustomAlert.tsx

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,11 @@ import { SPACING, TYPOGRAPHY } from '../constants';
1313
// AlertState + its factories are pure and live in utils/alertState so non-UI code can build an
1414
// alert state without importing this component (no-backward-layering-core). Re-exported here so
1515
// existing `from '../components/CustomAlert'` importers are unchanged.
16-
import {
17-
AlertButton,
18-
AlertState,
19-
initialAlertState,
20-
showAlert,
21-
hideAlert,
22-
} from '../utils/alertState';
16+
import { AlertButton } from '../utils/alertState';
2317

24-
export type { AlertButton, AlertState };
25-
export { initialAlertState, showAlert, hideAlert };
18+
export type { AlertButton };
19+
export type { AlertState } from '../utils/alertState';
20+
export { initialAlertState, showAlert, hideAlert } from '../utils/alertState';
2621

2722
export interface CustomAlertProps {
2823
visible: boolean;

src/screens/ChatScreen/useChatMessageHandlers.ts

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,38 @@ type RetryParams = {
2121
setDebugInfo: SetState<any>;
2222
};
2323

24+
/** Recorded modality when retrying an ASSISTANT message: its own output is the fact (an image
25+
* attachment → image turn); otherwise it's a text turn when it had a preceding user message. */
26+
function assistantRetryKind(message: Message, prevUser: Message | null): TurnKind | undefined {
27+
if (messageHasImageOutput(message)) return 'image';
28+
return prevUser ? 'text' : undefined;
29+
}
30+
31+
/** Shared context for the retry-branch helpers (bundled so each stays within the param limit). */
32+
type RetryCtx = { message: Message; genDeps: GenerationDeps; p: RetryParams; convId: string; msgs: Message[] };
33+
34+
/** Retry from a USER message: read the turn's recorded modality BEFORE deleting the reply that
35+
* carries it, so resend re-runs the SAME pipeline (deterministic) instead of re-classifying. */
36+
async function retryFromUserMessage({ message, genDeps, p, convId, msgs }: RetryCtx): Promise<void> {
37+
const idx = msgs.findIndex((m: Message) => m.id === message.id);
38+
const recordedKind = recordedTurnKind(msgs, message.id);
39+
logger.log(`[RESEND-SM] retry user msg idx=${idx} willDelete=${idx !== -1 && idx < msgs.length - 1} recordedKind=${recordedKind ?? 'none'}`);
40+
if (idx !== -1 && idx < msgs.length - 1) p.deleteMessagesAfter(convId, message.id);
41+
await regenerateResponseFn(genDeps, { setDebugInfo: p.setDebugInfo, userMessage: message, recordedKind });
42+
}
43+
44+
/** Retry from an ASSISTANT message: regenerate the preceding user turn with the recorded kind. */
45+
async function retryFromAssistantMessage({ message, genDeps, p, convId, msgs }: RetryCtx): Promise<void> {
46+
const idx = msgs.findIndex((m: Message) => m.id === message.id);
47+
const prev = idx > 0 ? msgs.slice(0, idx).reverse().find((m: Message) => m.role === 'user') ?? null : null;
48+
const recordedKind = assistantRetryKind(message, prev);
49+
logger.log(`[RESEND-SM] retry assistant msg idx=${idx} prevUser=${prev?.id ?? 'none'} recordedKind=${recordedKind ?? 'none'}`);
50+
if (prev) {
51+
p.deleteMessagesAfter(convId, prev.id);
52+
await regenerateResponseFn(genDeps, { setDebugInfo: p.setDebugInfo, userMessage: prev, recordedKind });
53+
}
54+
}
55+
2456
export async function handleRetryMessageFn(
2557
message: Message, genDeps: GenerationDeps, p: RetryParams,
2658
): Promise<void> {
@@ -39,25 +71,9 @@ export async function handleRetryMessageFn(
3971
if (!p.activeConversationId) { logger.log('[RESEND-SM] retry BAIL: no conv'); return; }
4072
// Stop any in-flight TTS before deleting messages (no-op without pro audio)
4173
callHook(HOOKS.audioStop);
42-
if (message.role === 'user') {
43-
const idx = msgs.findIndex((m: Message) => m.id === message.id);
44-
// Read the turn's recorded modality BEFORE deleting the reply that carries it, so resend
45-
// re-runs the SAME pipeline (deterministic) instead of re-classifying — the image-resend fix.
46-
const recordedKind = recordedTurnKind(msgs, message.id);
47-
logger.log(`[RESEND-SM] retry user msg idx=${idx} willDelete=${idx !== -1 && idx < msgs.length - 1} recordedKind=${recordedKind ?? 'none'}`);
48-
if (idx !== -1 && idx < msgs.length - 1) p.deleteMessagesAfter(p.activeConversationId, message.id);
49-
await regenerateResponseFn(genDeps, { setDebugInfo: p.setDebugInfo, userMessage: message, recordedKind });
50-
} else {
51-
const idx = msgs.findIndex((m: Message) => m.id === message.id);
52-
const prev = idx > 0 ? msgs.slice(0, idx).reverse().find((m: Message) => m.role === 'user') : null;
53-
// The retried assistant message IS the turn's reply — its output determines the kind directly.
54-
const recordedKind: TurnKind | undefined = messageHasImageOutput(message) ? 'image' : (prev ? 'text' : undefined);
55-
logger.log(`[RESEND-SM] retry assistant msg idx=${idx} prevUser=${prev?.id ?? 'none'} recordedKind=${recordedKind ?? 'none'}`);
56-
if (prev) {
57-
p.deleteMessagesAfter(p.activeConversationId, prev.id);
58-
await regenerateResponseFn(genDeps, { setDebugInfo: p.setDebugInfo, userMessage: prev, recordedKind });
59-
}
60-
}
74+
const ctx: RetryCtx = { message, genDeps, p, convId: p.activeConversationId, msgs };
75+
if (message.role === 'user') await retryFromUserMessage(ctx);
76+
else await retryFromAssistantMessage(ctx);
6177
}
6278

6379
type EditParams = {

src/screens/ChatScreen/useChatScreen.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,16 @@ import { RootStackParamList } from '../../navigation/types';
1818
import { ensureModelLoadedFn, ensureTextModelForChatFn, handleModelSelectFn, handleUnloadModelFn, initiateModelLoad, useChatImageModelEffects, useChatModelStateSync } from './useChatModelActions';
1919
import { startGenerationFn, handleSendFn, handleStopFn, handleSelectProjectFn, dispatchGenerationFn } from './useChatGenerationActions';
2020
import { handleRetryMessageFn, handleEditMessageFn, handleDeleteConversationFn, handleGenerateImageFromMsgFn } from './useChatMessageHandlers';
21-
import { getDisplayMessages, getPlaceholderText, ChatMessageItem } from './types';
21+
import { getDisplayMessages } from './types';
2222
import { saveImageToGallery } from './useSaveImage';
2323
import {
2424
isSuspiciousRecoveredImageModel,
2525
isSuspiciousRecoveredTextModel,
2626
} from '../../utils/modelSelectorFilters';
2727

28-
export type { AlertState, ChatMessageItem };
29-
export { getPlaceholderText };
28+
export type { AlertState };
29+
export type { ChatMessageItem } from './types';
30+
export { getPlaceholderText } from './types';
3031

3132
type ChatScreenRouteProp = RouteProp<RootStackParamList, 'Chat'>;
3233

src/screens/ModelsScreen/imageDownloadQnn.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { hideAlert, showAlert } from '../../components/CustomAlert';
2-
import { ImageModelDescriptor } from './types';
3-
import { ImageDownloadDeps } from './types';
2+
import { ImageModelDescriptor, ImageDownloadDeps } from './types';
43

54
export function getQnnWarningMessage(
65
modelInfo: ImageModelDescriptor,

src/services/engines.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@ export interface EngineCapabilities {
1616
}
1717

1818
/** Runtime inputs for deriveEngineCapabilities — passed explicitly so the rule is pure/testable. */
19+
/** A remote model's declared capabilities (single named type so callers don't index CapabilityInputs). */
20+
export type RemoteCaps = { supportsVision?: boolean; supportsToolCalling?: boolean; supportsThinking?: boolean } | null;
21+
1922
export interface CapabilityInputs {
2023
/** A remote (gateway) model is active — its declared capabilities win. */
2124
isRemote: boolean;
22-
remoteCaps?: { supportsVision?: boolean; supportsToolCalling?: boolean; supportsThinking?: boolean } | null;
25+
remoteCaps?: RemoteCaps;
2326
/** The active LOCAL model's engine ('litert' | 'llama' | ...) and LiteRT capability flags. */
2427
engine?: string;
2528
liteRTVision?: boolean;
@@ -113,7 +116,7 @@ export function isModelReady(model: { engine?: string; filePath?: string } | nul
113116
*/
114117
export function activeTextCapabilities(i: {
115118
isRemote: boolean;
116-
remoteCaps?: CapabilityInputs['remoteCaps'];
119+
remoteCaps?: RemoteCaps;
117120
model: DownloadedModel | null | undefined;
118121
}): EngineCapabilities {
119122
const litert = i.model && isLiteRTModel(i.model) ? i.model : null;

src/stores/downloadStore.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,11 @@ import logger from '../utils/logger';
44
// The status classification + DownloadEntry shape live in a PURE util so pure consumers can use
55
// them without depending on this store (utils-stay-pure). Re-exported here for back-compat — every
66
// existing `from '../stores/downloadStore'` importer of these keeps working unchanged.
7-
import {
8-
DownloadStatus,
9-
ModelType,
10-
DownloadEntry,
11-
isActiveStatus,
12-
isQueuedStatus,
13-
isDownloadingStatus,
14-
} from '../utils/downloadStatus';
7+
import { DownloadStatus, DownloadEntry } from '../utils/downloadStatus';
158

16-
export type { DownloadStatus, ModelType, DownloadEntry };
17-
export { isActiveStatus, isQueuedStatus, isDownloadingStatus };
9+
export type { DownloadStatus, DownloadEntry };
10+
export type { ModelType } from '../utils/downloadStatus';
11+
export { isActiveStatus, isQueuedStatus, isDownloadingStatus } from '../utils/downloadStatus';
1812

1913
interface DownloadStoreState {
2014
downloads: Record<ModelKey, DownloadEntry>

0 commit comments

Comments
 (0)