Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 53 additions & 1 deletion apps/mobile/src/components/agents/markdown-renderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import Module from 'node:module';
import { createElement, type ReactElement, type ReactNode } from 'react';
import { act, TestRenderer } from '@/test/renderer';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { confirmAndOpenMarkdownLink } from './markdown-link-confirm';

Expand Down Expand Up @@ -920,3 +920,55 @@ describe('MarkdownRenderer nested table fallback', () => {
});
});
});

/**
* Proof for the explorer's `session-tools-ar` finding: in the Arabic interface
* the assistant body and its bullet list read as centre-aligned, with the
* marker detached at the right edge (`session-tools-ar.png`). React Native only
* centres a paragraph through `textAlign`, so the renderer's contract is that
* the RTL path names the base direction (`writingDirection: 'rtl'`, from
* `lib/rtl-text.ts`) and no paragraph or marker style carries a `textAlign` at
* all. The marker box is the library's own; the renderer never gives it one.
*/
describe('MarkdownRenderer RTL paragraph direction (explorer session-tools-ar)', () => {
afterEach(() => {
rnStub.I18nManager.isRTL = false;
});

function elementStyle(element: ReactElement): Record<string, unknown> {
return flattenStyle((element.props as { style?: unknown }).style);
}

it('names the paragraph base direction without centring it', async () => {
rnStub.I18nManager.isRTL = true;
const renderer = await createRenderer();
const heading = renderer.heading('Why fork PRs can fail', { fontSize: 20 }) as ReactElement;
const body = renderer.escape('Secrets are withheld.', { fontSize: 16 }) as ReactElement;
const link = renderer.link('catalog.json', 'https://example.com', {
fontSize: 16,
}) as ReactElement;
for (const element of [heading, body, link]) {
const style = elementStyle(element);
expect(style.writingDirection).toBe('rtl');
expect(style.textAlign).toBeUndefined();
}
});

it('leaves the LTR path without a writing direction or alignment', async () => {
const renderer = await createRenderer();
const body = renderer.escape('Secrets are withheld.', { fontSize: 16 }) as ReactElement;
const style = elementStyle(body);
expect(style.writingDirection).toBeUndefined();
expect(style.textAlign).toBeUndefined();
});

it('has no centring style on the paragraph or list marker box', async () => {
const { getMarkdownStyles } = await import('./markdown-palette');
const styles = getMarkdownStyles(palette);
for (const key of ['text', 'paragraph', 'list', 'li'] as const) {
const style = flattenStyle(styles[key]);
expect(style.textAlign).toBeUndefined();
expect(style.alignItems).toBeUndefined();
}
});
});
24 changes: 19 additions & 5 deletions apps/mobile/src/components/agents/message-failure-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,24 @@ describe('selectMessageFailure', () => {
});

describe('delivery', () => {
it('maps every reason to fixed copy and never emits raw error text', () => {
it('maps every delivery reason to fixed copy and never emits raw error text', () => {
const cases = [
{ reason: 'interrupted', detail: 'You stopped this message.' },
{ reason: 'interrupted', title: 'Failed to deliver', detail: 'You stopped this message.' },
{
reason: 'exhausted',
title: 'Failed to deliver',
detail: 'We could not deliver this message after several attempts.',
},
{ reason: 'execution', detail: 'The agent could not run this message.' },
] as const;

for (const { reason, detail } of cases) {
for (const { reason, title, detail } of cases) {
const result = selectMessageFailure({
info: userInfo(),
deliveryState: { status: 'failed', error: 'RAW_TRANSPORT_TEXT', reason },
});
expect(result).not.toBeNull();
expect(result?.kind).toBe('delivery');
expect(result?.title).toBe('Failed to deliver');
expect(result?.title).toBe(title);
expect(result?.detail).toBe(detail);
expect(result?.title).not.toContain('RAW_TRANSPORT_TEXT');
expect(result?.detail).not.toContain('RAW_TRANSPORT_TEXT');
Expand All @@ -83,6 +83,20 @@ describe('selectMessageFailure', () => {
}
});

it('states an agent-execution failure once, in the assistant-failure copy', () => {
const result = selectMessageFailure({
info: userInfo(),
deliveryState: { status: 'failed', error: 'RAW_TRANSPORT_TEXT', reason: 'execution' },
});
expect(result).not.toBeNull();
expect(result?.kind).toBe('delivery');
expect(result?.title).toBe('Response failed');
expect(result?.detail).toBeNull();
expect(result?.copyDetail).toBe('RAW_TRANSPORT_TEXT');
expect(result?.canRetry).toBe(true);
expect(result?.canCopy).toBe(true);
});

it('exposes no copy detail when the delivery failure carries no transport text', () => {
const result = selectMessageFailure({
info: userInfo(),
Expand Down
76 changes: 62 additions & 14 deletions apps/mobile/src/components/agents/message-failure-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,24 @@ type DeliveryReason = Extract<MessageDeliveryState, { status: 'failed' }>['reaso
const DELIVERY_DETAIL_KEY_BY_REASON = {
interrupted: 'agentChat.messageFailure.deliveryInterrupted',
exhausted: 'agentChat.messageFailure.deliveryExhausted',
// `execution` is the response failure, not a delivery one (the message
// reached the agent): the row shows the assistant-failure title instead, so
// this line is never rendered. It stays in the record to keep the reason
// coverage complete.
execution: 'agentChat.messageFailure.deliveryExecution',
} as const satisfies Record<DeliveryReason, string>;

/**
* The one delivery reason that is an agent run failing, not the transport: the
* message was delivered and the agent could not run it. The row states it in
* the assistant-failure copy ("Response failed") with no second line — the
* session's own status line reports the same failure, so a delivery-flavoured
* title here plus the footer's line read as the failure stated three times
* (UX-DEFECT session-detail failed turn). Retry and Copy-to-composer keep
* working: the transport text stays reachable behind the copy action.
*/
const AGENT_EXECUTION_DELIVERY_REASON: DeliveryReason = 'execution';

/**
* Assistant error names that can never be retried. Pinned to the exact names
* in `packages/app-shared/src/opencode.gen.ts`.
Expand All @@ -25,22 +40,22 @@ export const NON_RETRYABLE_ASSISTANT_ERRORS: readonly string[] = [
];

/**
* Fixed, safe copy for a known assistant error name. An unknown name has no
* line of its own: the title already states that the response failed, so the
* footer adds no detail rather than repeating the title in a sentence
* (`messageFailure.assistantFailed` is the fixed footer's line, not the
* message row's). Never surfaces `error.data` or provider message text.
* The catalog key for a known assistant error name's fixed, safe detail line.
* An unknown name has no line of its own: the title already states that the
* response failed, so the footer adds no detail rather than repeating the title
* in a sentence (`messageFailure.assistantFailed` is the fixed footer's line,
* not the message row's). Never surfaces `error.data` or provider message text.
*/
function assistantDetail(errorName: string): string | null {
function assistantDetailKey(errorName: string): string | null {
switch (errorName) {
case 'ProviderAuthError': {
return i18n.t('agentChat.messageFailure.assistantProviderRejected');
return 'agentChat.messageFailure.assistantProviderRejected';
}
case 'MessageAbortedError': {
return i18n.t('agentChat.messageFailure.assistantStopped');
return 'agentChat.messageFailure.assistantStopped';
}
case 'ContextOverflowError': {
return i18n.t('agentChat.messageFailure.assistantContextOverflow');
return 'agentChat.messageFailure.assistantContextOverflow';
}
default: {
return null;
Expand All @@ -50,11 +65,26 @@ function assistantDetail(errorName: string): string | null {

export type MessageFailure = {
kind: 'delivery' | 'assistant';
/**
* The catalog key behind `title`. The duplicate check compares keys, not the
* resolved copy: `selectMessageFailure` runs inside a memo keyed on the
* message arrays, so an in-place language switch would otherwise leave a
* stale-language title that no longer matches the footer copy resolved at
* render time.
*/
titleKey: string;
title: string;
/**
* The explanation line under the title, or `null` when the title alone says
* it (an assistant failure with no classified reason). The footer then shows
* one statement plus the action rather than the same sentence twice.
* The catalog key behind `detail`, or `null` when the title alone says it (an
* assistant failure with no classified reason, or an agent-execution delivery
* failure whose response-failure title is the whole statement). The footer
* then shows one statement plus the action rather than the same sentence
* twice.
*/
detailKey: string | null;
/**
* `detailKey` resolved at selection time, for rendering. `null` exactly when
* `detailKey` is `null`.
*/
detail: string | null;
/**
Expand All @@ -75,10 +105,25 @@ export function selectMessageFailure(input: {
const { deliveryState, info } = input;

if (info.role === 'user' && deliveryState?.status === 'failed') {
if (deliveryState.reason === AGENT_EXECUTION_DELIVERY_REASON) {
return {
kind: 'delivery',
titleKey: 'agentChat.messageFailure.assistantTitle',
title: i18n.t('agentChat.messageFailure.assistantTitle'),
detailKey: null,
detail: null,
copyDetail: deliveryState.error,
canRetry: true,
canCopy: true,
};
}
const detailKey = DELIVERY_DETAIL_KEY_BY_REASON[deliveryState.reason];
return {
kind: 'delivery',
titleKey: 'agentChat.messageFailure.deliveryTitle',
title: i18n.t('agentChat.messageFailure.deliveryTitle'),
detail: i18n.t(DELIVERY_DETAIL_KEY_BY_REASON[deliveryState.reason]),
detailKey,
detail: i18n.t(detailKey),
copyDetail: deliveryState.error,
canRetry: true,
canCopy: true,
Expand All @@ -87,10 +132,13 @@ export function selectMessageFailure(input: {

if (info.role === 'assistant' && info.error) {
const errorName = info.error.name;
const detailKey = assistantDetailKey(errorName);
return {
kind: 'assistant',
titleKey: 'agentChat.messageFailure.assistantTitle',
title: i18n.t('agentChat.messageFailure.assistantTitle'),
detail: assistantDetail(errorName),
detailKey,
detail: detailKey === null ? null : i18n.t(detailKey),
copyDetail: '',
canRetry: !NON_RETRYABLE_ASSISTANT_ERRORS.includes(errorName),
canCopy: false,
Expand Down
65 changes: 65 additions & 0 deletions apps/mobile/src/components/agents/model-selector-label.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { describe, expect, it } from 'vitest';

import { resolveModelSelectorLabel } from './model-selector-label';

const FALLBACK = 'Model';

describe('resolveModelSelectorLabel', () => {
it('uses the matched catalog option name', () => {
expect(
resolveModelSelectorLabel({
selectedName: 'DeepSeek V4 Flash 0731',
value: 'deepseek/deepseek-v4-flash-0731',
providerAware: false,
fallbackLabel: FALLBACK,
})
).toBe('DeepSeek V4 Flash 0731');
});

it('strips the vendor prefix from an unmatched stored model reference', () => {
// The chip used to print this raw string verbatim (explorer
// session-typed-kb-up): it repeated the vendor as
// `DeepSeek: DeepSeek V4 Flash 0731`.
expect(
resolveModelSelectorLabel({
selectedName: undefined,
value: 'DeepSeek: DeepSeek V4 Flash 0731',
providerAware: false,
fallbackLabel: FALLBACK,
})
).toBe('DeepSeek V4 Flash 0731');
});

it('keeps an unmatched value that carries no vendor prefix', () => {
expect(
resolveModelSelectorLabel({
selectedName: undefined,
value: 'custom-model-id',
providerAware: false,
fallbackLabel: FALLBACK,
})
).toBe('custom-model-id');
});

it('falls back to the generic label when the value is empty', () => {
expect(
resolveModelSelectorLabel({
selectedName: undefined,
value: '',
providerAware: false,
fallbackLabel: FALLBACK,
})
).toBe(FALLBACK);
});

it('falls back to the generic label for a provider-aware catalog miss', () => {
expect(
resolveModelSelectorLabel({
selectedName: undefined,
value: 'DeepSeek: DeepSeek V4 Flash 0731',
providerAware: true,
fallbackLabel: FALLBACK,
})
).toBe(FALLBACK);
});
});
39 changes: 39 additions & 0 deletions apps/mobile/src/components/agents/model-selector-label.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { formatShortModelDisplayName } from '@/lib/model-display-name';

type ModelSelectorLabelInput = {
/** The catalog option's display name, when the value matches one. */
selectedName: string | undefined;
/** The session's stored model reference or id. */
value: string;
/**
* True when the options carry CLI-catalog refs. The chip then cannot know a
* human name for an unmatched value and falls back to the generic label.
*/
providerAware: boolean;
/** `t('common.model')`: the label when nothing else can be resolved. */
fallbackLabel: string;
};

/**
* The label for the `ModelSelector` chip.
*
* A session can report a model the local catalog does not carry (a CLI build
* ahead of the app's list), and the stored reference then arrives as the full
* `<Vendor>: <Model>` display name. Printing it verbatim repeats the vendor in
* user-facing copy — the chip read `DeepSeek: DeepSeek V4 Flash 0731` (explorer
* session-typed-kb-up) — so the chip falls back to the same short name the
* model picker shows. Mirrors the web chat toolbar, which never renders the raw
* model reference.
*/
export function resolveModelSelectorLabel(input: ModelSelectorLabelInput): string {
if (input.selectedName !== undefined) {
return input.selectedName;
}
if (!input.providerAware && input.value !== '') {
const shortName = formatShortModelDisplayName(input.value);
if (shortName !== '') {
return shortName;
}
}
return input.fallbackLabel;
}
17 changes: 13 additions & 4 deletions apps/mobile/src/components/agents/model-selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { modelPickerSlot } from '@/lib/route-registry';
import { cn } from '@/lib/utils';

import { modelSelectorBadges } from './model-selector-badges';
import { resolveModelSelectorLabel } from './model-selector-label';

type ModelSelectorProps = {
value: string;
Expand Down Expand Up @@ -151,10 +152,18 @@ export function ModelSelector({
const providerAware = pickerOptions.some(
option => option.modelRef !== undefined || !option.showGatewayMetadata
);
const fallbackLabel = !providerAware && value ? value : t('common.model');
const label = selectedModel
? autoModelLabel(selectedModel.displayId, selectedModel.name)
: fallbackLabel;
// A matched option can be one of Kilo's own Auto models, whose backend name no
// catalog translates, so resolve its label first and let the helper fall back
// to the short name (or the generic label) for a stored reference the catalog
// does not carry.
const label = resolveModelSelectorLabel({
selectedName: selectedModel
? autoModelLabel(selectedModel.displayId, selectedModel.name)
: undefined,
value,
providerAware,
fallbackLabel: t('common.model'),
});
const { byok, collectsData } = modelSelectorBadges(selectedModel);
const hasVariants = selectedModel ? selectedModel.variants.length > 1 : false;
const variantLabel = variant ? thinkingEffortLabel(variant) : '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
} from '@/lib/run-on-destination';
import { shouldShowRunOnSelector } from '@/lib/should-show-run-on-selector';
import { peekSharePayload } from '@/lib/share-payload';
import { sessionDisplayTitle } from '@/lib/session-display-title';
import { useNewSessionShareRemote } from '@/lib/use-new-session-share-remote';
import { useNewSessionRepos } from '@/lib/use-new-session-repos';
import { useTRPC } from '@/lib/trpc';
Expand Down Expand Up @@ -720,7 +721,7 @@ export function NewSessionScreenBody() {
<View className="px-4 pt-4">
<Text className="text-sm text-muted-foreground">
{t('agentChat.newSession.continueFrom', {
title: cloneSourceTitle || t('agentChat.session.title'),
title: sessionDisplayTitle(cloneSourceTitle) ?? t('agentChat.session.title'),
})}
</Text>
</View>
Expand Down
Loading
Loading