Skip to content

fix(mobile): surface attachment problems in the composer (port #3838) #3865

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

Closed
Gilbert09 wants to merge 1 commit into from
Closed

fix(mobile): surface attachment problems in the composer (port #3838) #3865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 28 additions & 2 deletions apps/mobile/src/app/task/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import {
Sparkle,
StopIcon,
} from "phosphor-react-native";
import { useCallback, useState } from "react";
import { useCallback, useMemo, useState } from "react";
import {
ActivityIndicator,
Alert,
Pressable,
ScrollView,
TextInput,
Expand All @@ -43,6 +44,7 @@ import {
pickPhotoFromLibrary,
} from "@/features/tasks/composer/attachments/pickers";
import type { PendingAttachment } from "@/features/tasks/composer/attachments/types";
import { validateAttachment } from "@/features/tasks/composer/attachments/validation";
import { DotBackground } from "@/features/tasks/composer/DotBackground";
import {
DEFAULT_EXECUTION_MODE,
Expand Down Expand Up @@ -205,6 +207,19 @@ export default function NewTaskScreen() {
const [attachments, setAttachments] = useState<PendingAttachment[]>([]);
const [attachmentSheetOpen, setAttachmentSheetOpen] = useState(false);

// Errors are a pure function of the picked attachments, so derive rather
// than mirror them in state.
const attachmentErrors = useMemo(
() =>
Object.fromEntries(
attachments.flatMap((att) => {
const reason = validateAttachment(att);
return reason ? [[att.id, reason]] : [];
}),
),
[attachments],
);

const appendTranscript = useCallback((transcript: string) => {
setPrompt((prev) => (prev ? `${prev} ${transcript}` : transcript));
}, []);
Expand Down Expand Up @@ -236,7 +251,10 @@ export default function NewTaskScreen() {
async (picker: () => Promise<PendingAttachment | null>) => {
try {
const att = await picker();
if (att) setAttachments((prev) => [...prev, att]);
if (!att) return;
setAttachments((prev) => [...prev, att]);
const reason = validateAttachment(att);
if (reason) Alert.alert("Attachment problem", reason);
} catch (err) {
log.error("Failed to pick attachment", err);
}
Expand Down Expand Up @@ -267,6 +285,13 @@ export default function NewTaskScreen() {
if (!hasContent || !isRepositorySelectionComplete(selection) || creating) {
return;
}
if (attachments.some((att) => validateAttachment(att))) {
Alert.alert(
"Attachment can't be sent",
"Remove the highlighted attachment before sending.",
);
return;
}

setCreating(true);

Expand Down Expand Up @@ -560,6 +585,7 @@ export default function NewTaskScreen() {
<AttachmentsBar
attachments={attachments}
onRemove={removeAttachment}
errors={attachmentErrors}
/>
<TextInput
className="px-4 pt-3.5 pb-3 text-[15px] text-gray-12"
Expand Down
29 changes: 28 additions & 1 deletion apps/mobile/src/features/tasks/composer/TaskChatComposer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ import {
type ReactNode,
useCallback,
useEffect,
useMemo,
useRef,
useState,
} from "react";
import {
ActivityIndicator,
Alert,
Animated,
Easing,
Keyboard,
Expand All @@ -43,6 +45,7 @@ import {
pickPhotoFromLibrary,
} from "./attachments/pickers";
import type { PendingAttachment } from "./attachments/types";
import { validateAttachment } from "./attachments/validation";
import {
DEFAULT_EXECUTION_MODE,
DEFAULT_MODEL,
Expand Down Expand Up @@ -187,6 +190,19 @@ export function TaskChatComposer({
const [attachments, setAttachments] = useState<PendingAttachment[]>([]);
const [attachmentSheetOpen, setAttachmentSheetOpen] = useState(false);

// Errors are a pure function of the picked attachments, so derive rather
// than mirror them in state.
const attachmentErrors = useMemo(
() =>
Object.fromEntries(
attachments.flatMap((att) => {
const reason = validateAttachment(att);
return reason ? [[att.id, reason]] : [];
}),
),
[attachments],
);

// Mirror composer state into refs so a failed send can read the current
// value after awaiting, rather than the value captured when it was sent.
const messageRef = useRef(message);
Expand Down Expand Up @@ -234,6 +250,13 @@ export function TaskChatComposer({

const handleSend = () => {
if (!hasContent || disabled) return;
if (attachments.some((att) => validateAttachment(att))) {
Alert.alert(
"Attachment can't be sent",
"Remove the highlighted attachment before sending.",
);
return;
}
const submitted: ComposerContent = { text: message.trim(), attachments };
const submissionId = ++submissionRef.current;
Keyboard.dismiss();
Expand All @@ -256,7 +279,10 @@ export function TaskChatComposer({
) => {
try {
const att = await picker();
if (att) setAttachments((prev) => [...prev, att]);
if (!att) return;
setAttachments((prev) => [...prev, att]);
const reason = validateAttachment(att);
if (reason) Alert.alert("Attachment problem", reason);
} catch (err) {
log.error("Failed to pick attachment", err);
}
Expand Down Expand Up @@ -325,6 +351,7 @@ export function TaskChatComposer({
<AttachmentsBar
attachments={attachments}
onRemove={removeAttachment}
errors={attachmentErrors}
/>
<TextInput
className="px-4 pt-3.5 pb-3 text-[15px] text-gray-12"
Expand Down
97 changes: 60 additions & 37 deletions apps/mobile/src/features/tasks/composer/attachments/AttachmentsBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { Text } from "@components/text";
import { FileText, X } from "phosphor-react-native";
import { FileText, WarningCircle, X } from "phosphor-react-native";
import { Image, Pressable, ScrollView, View } from "react-native";
import { useThemeColors } from "@/lib/theme";
import type { PendingAttachment } from "./types";

interface AttachmentsBarProps {
attachments: PendingAttachment[];
onRemove: (id: string) => void;
/** Reason keyed by attachment id when it failed pick-time validation. */
errors?: Record<string, string>;
}

function truncate(name: string, max = 18): string {
Expand All @@ -18,7 +20,11 @@ function truncate(name: string, max = 18): string {
return `${name.slice(0, max - 1)}…`;
}

export function AttachmentsBar({ attachments, onRemove }: AttachmentsBarProps) {
export function AttachmentsBar({
attachments,
onRemove,
errors,
}: AttachmentsBarProps) {
const themeColors = useThemeColors();
if (attachments.length === 0) return null;

Expand All @@ -33,43 +39,60 @@ export function AttachmentsBar({ attachments, onRemove }: AttachmentsBarProps) {
gap: 8,
}}
>
{attachments.map((att) => (
<View
key={att.id}
className="relative h-16 rounded-lg border border-gray-6 bg-gray-2"
style={{ minWidth: 64 }}
>
{att.kind === "image" ? (
<Image
source={{ uri: att.uri }}
className="h-16 w-16 rounded-lg"
resizeMode="cover"
/>
) : (
<View className="h-16 w-32 flex-row items-center gap-2 px-2">
<FileText
size={20}
color={themeColors.gray[11]}
weight="regular"
{attachments.map((att) => {
const error = errors?.[att.id];
return (
<View
key={att.id}
className={`relative h-16 rounded-lg border bg-gray-2 ${
error ? "border-status-error" : "border-gray-6"
}`}
style={{ minWidth: 64 }}
>
{att.kind === "image" ? (
<Image
source={{ uri: att.uri }}
className="h-16 w-16 rounded-lg"
resizeMode="cover"
/>
<Text
className="flex-1 text-[12px] text-gray-12"
numberOfLines={2}
) : (
<View className="h-16 w-32 flex-row items-center gap-2 px-2">
<FileText
size={20}
color={themeColors.gray[11]}
weight="regular"
/>
<Text
className="flex-1 text-[12px] text-gray-12"
numberOfLines={2}
>
{truncate(att.fileName, 22)}
</Text>
</View>
)}
{error ? (
<View
accessibilityLabel={`Attachment can't be sent: ${error}`}
className="absolute inset-0 items-center justify-center rounded-lg bg-black/40"
>
{truncate(att.fileName, 22)}
</Text>
</View>
)}
<Pressable
onPress={() => onRemove(att.id)}
hitSlop={8}
accessibilityLabel={`Remove ${att.fileName}`}
className="-top-1.5 -right-1.5 absolute h-5 w-5 items-center justify-center rounded-full bg-gray-12 active:opacity-80"
>
<X size={12} color={themeColors.background} weight="bold" />
</Pressable>
</View>
))}
<WarningCircle
size={20}
color={themeColors.status.error}
weight="fill"
/>
</View>
) : null}
<Pressable
onPress={() => onRemove(att.id)}
hitSlop={8}
accessibilityLabel={`Remove ${att.fileName}`}
className="-top-1.5 -right-1.5 absolute h-5 w-5 items-center justify-center rounded-full bg-gray-12 active:opacity-80"
>
<X size={12} color={themeColors.background} weight="bold" />
</Pressable>
</View>
);
})}
</ScrollView>
);
}
78 changes: 8 additions & 70 deletions apps/mobile/src/features/tasks/composer/attachments/buildCloudPrompt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,73 +1,16 @@
import {
estimateBase64Bytes,
getFileExtension,
MAX_CLAUDE_IMAGE_BYTES,
} from "@posthog/shared";
import * as FileSystem from "expo-file-system/legacy";
import type { CloudPromptBlock, PendingAttachment } from "./types";
import { isTextAttachment } from "./validation";

const MAX_EMBEDDED_TEXT_CHARS = 100_000;
const MAX_EMBEDDED_IMAGE_BYTES = 5 * 1024 * 1024;

const TEXT_MIME_PREFIXES = ["text/"];
const TEXT_MIME_TYPES = new Set([
"application/json",
"application/xml",
"application/javascript",
"application/typescript",
"application/x-sh",
"application/x-yaml",
"application/x-toml",
]);
const TEXT_EXTENSIONS = new Set([
"c",
"cc",
"cfg",
"conf",
"cpp",
"cs",
"css",
"csv",
"env",
"gitignore",
"go",
"h",
"hpp",
"html",
"ini",
"java",
"js",
"json",
"jsx",
"log",
"md",
"mjs",
"py",
"rb",
"rs",
"scss",
"sh",
"sql",
"svg",
"toml",
"ts",
"tsx",
"txt",
"xml",
"yaml",
"yml",
"zsh",
]);

function getExt(fileName: string): string {
const dot = fileName.lastIndexOf(".");
return dot >= 0 ? fileName.slice(dot + 1).toLowerCase() : "";
}

function isTextAttachment(mimeType: string, fileName: string): boolean {
const mt = mimeType.toLowerCase();
if (TEXT_MIME_PREFIXES.some((p) => mt.startsWith(p))) return true;
if (TEXT_MIME_TYPES.has(mt)) return true;
return TEXT_EXTENSIONS.has(getExt(fileName));
}

function getTextMimeType(fileName: string, fallback: string): string {
const ext = getExt(fileName);
const ext = getFileExtension(fileName);
switch (ext) {
case "json":
return "application/json";
Expand All @@ -87,17 +30,12 @@ function truncateText(text: string): string {
return `${text.slice(0, MAX_EMBEDDED_TEXT_CHARS)}\n\n[Attachment truncated to ${MAX_EMBEDDED_TEXT_CHARS.toLocaleString()} characters for this cloud prompt.]`;
}

function estimateBase64Bytes(base64: string): number {
const padding = base64.endsWith("==") ? 2 : base64.endsWith("=") ? 1 : 0;
return Math.floor((base64.length * 3) / 4) - padding;
}

async function buildBlock(att: PendingAttachment): Promise<CloudPromptBlock> {
if (att.kind === "image") {
const base64 = await FileSystem.readAsStringAsync(att.uri, {
encoding: FileSystem.EncodingType.Base64,
});
if (estimateBase64Bytes(base64) > MAX_EMBEDDED_IMAGE_BYTES) {
if (estimateBase64Bytes(base64) > MAX_CLAUDE_IMAGE_BYTES) {
throw new Error(
`${att.fileName} is too large for a cloud image attachment (max 5 MB).`,
);
Expand Down
Loading
Loading