fixed review comments

This commit is contained in:
Arjun 2026-02-26 21:03:58 +05:30
parent 5d78d66b00
commit a0a7933a7c
8 changed files with 53 additions and 76 deletions

View file

@ -1180,7 +1180,10 @@ function App() {
const contentParts = msg.content as Array<{
type: string
text?: string
attachment?: NonNullable<ChatMessage['attachments']>[number]
path?: string
filename?: string
mimeType?: string
size?: number
toolCallId?: string
toolName?: string
arguments?: ToolUIPart['input']
@ -1191,13 +1194,16 @@ function App() {
.map((part) => part.text || '')
.join('')
const attachmentParts = contentParts.filter((part) => part.type === 'attachment' && part.attachment)
const attachmentParts = contentParts.filter((part) => part.type === 'attachment' && part.path)
if (attachmentParts.length > 0) {
msgAttachments = attachmentParts
.map((part) => part.attachment)
.filter((attachment): attachment is NonNullable<ChatMessage['attachments']>[number] => Boolean(attachment))
msgAttachments = attachmentParts.map((part) => ({
path: part.path!,
filename: part.filename || part.path!.split('/').pop() || part.path!,
mimeType: part.mimeType || 'application/octet-stream',
size: part.size,
}))
}
// Also extract tool-call parts from assistant messages
if (msg.role === 'assistant') {
for (const part of contentParts) {
@ -1654,10 +1660,9 @@ function App() {
const userMessageId = `user-${Date.now()}`
const displayAttachments: ChatMessage['attachments'] = hasAttachments
? stagedAttachments.map((attachment) => ({
type: attachment.isImage ? 'image' : 'file',
path: attachment.path,
filename: attachment.filename,
mediaType: attachment.mediaType,
mimeType: attachment.mimeType,
size: attachment.size,
thumbnailUrl: attachment.thumbnailUrl,
}))
@ -1697,13 +1702,10 @@ function App() {
| { type: 'text'; text: string }
| {
type: 'attachment'
attachment: {
type: 'file' | 'image'
path: string
filename: string
mediaType: string
size?: number
}
path: string
filename: string
mimeType: string
size?: number
}
const contentParts: ContentPart[] = []
@ -1712,12 +1714,9 @@ function App() {
for (const mention of mentions) {
contentParts.push({
type: 'attachment',
attachment: {
type: 'file',
path: mention.path,
filename: mention.displayName || mention.path.split('/').pop() || mention.path,
mediaType: 'text/markdown',
},
path: mention.path,
filename: mention.displayName || mention.path.split('/').pop() || mention.path,
mimeType: 'text/markdown',
})
}
}
@ -1725,13 +1724,10 @@ function App() {
for (const attachment of stagedAttachments) {
contentParts.push({
type: 'attachment',
attachment: {
type: attachment.isImage ? 'image' : 'file',
path: attachment.path,
filename: attachment.filename,
mediaType: attachment.mediaType,
size: attachment.size,
},
path: attachment.path,
filename: attachment.filename,
mimeType: attachment.mimeType,
size: attachment.size,
})
}

View file

@ -37,7 +37,7 @@ export type StagedAttachment = {
id: string
path: string
filename: string
mediaType: string
mimeType: string
isImage: boolean
size: number
thumbnailUrl?: string
@ -130,7 +130,7 @@ function ChatInputInner({
id: `att-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
path: filePath,
filename: getFileDisplayName(filePath),
mediaType: mime,
mimeType: mime,
isImage: image,
size: result.size,
thumbnailUrl: image ? `data:${mime};base64,${result.data}` : undefined,

View file

@ -54,13 +54,13 @@ function ImageAttachmentPreview({ attachment }: { attachment: MessageAttachment
() => async () => {
try {
const result = await window.ipc.invoke('shell:readFileBase64', { path: attachment.path })
const mimeType = result.mimeType || attachment.mediaType || 'image/*'
const mimeType = result.mimeType || attachment.mimeType || 'image/*'
setSrc(`data:${mimeType};base64,${result.data}`)
} catch {
// Keep current src; fallback rendering (broken image icon) is better than crashing.
}
},
[attachment.mediaType, attachment.path]
[attachment.mimeType, attachment.path]
)
useEffect(() => {
@ -91,8 +91,8 @@ interface ChatMessageAttachmentsProps {
export function ChatMessageAttachments({ attachments, className }: ChatMessageAttachmentsProps) {
if (attachments.length === 0) return null
const imageAttachments = attachments.filter((attachment) => isImageMime(attachment.mediaType))
const fileAttachments = attachments.filter((attachment) => !isImageMime(attachment.mediaType))
const imageAttachments = attachments.filter((attachment) => isImageMime(attachment.mimeType))
const fileAttachments = attachments.filter((attachment) => !isImageMime(attachment.mimeType))
return (
<div className={cn('flex flex-col items-end gap-2', className)}>

View file

@ -3,7 +3,7 @@ import { getExtension } from '@/lib/file-utils'
export type AttachmentLike = {
filename?: string
path: string
mediaType: string
mimeType: string
}
export type AttachmentIconKind =
@ -39,7 +39,7 @@ export function getAttachmentTypeLabel(attachment: AttachmentLike): string {
const ext = getExtension(getAttachmentDisplayName(attachment))
if (ext) return ext.toUpperCase()
const mediaType = attachment.mediaType.toLowerCase()
const mediaType = attachment.mimeType.toLowerCase()
if (mediaType.startsWith('audio/')) return 'AUDIO'
if (mediaType.startsWith('video/')) return 'VIDEO'
if (mediaType.startsWith('text/')) return 'TEXT'
@ -52,7 +52,7 @@ export function getAttachmentTypeLabel(attachment: AttachmentLike): string {
}
export function getAttachmentIconKind(attachment: AttachmentLike): AttachmentIconKind {
const mediaType = attachment.mediaType.toLowerCase()
const mediaType = attachment.mimeType.toLowerCase()
const ext = getExtension(attachment.filename || attachment.path)
if (mediaType.startsWith('audio/')) return 'audio'

View file

@ -3,10 +3,9 @@ import z from 'zod'
import { AskHumanRequestEvent, ToolPermissionRequestEvent } from '@x/shared/src/runs.js'
export interface MessageAttachment {
type: 'file' | 'image'
path: string
filename: string
mediaType: string
mimeType: string
size?: number
thumbnailUrl?: string
}

View file

@ -416,23 +416,19 @@ export function convertFromMessages(messages: z.infer<typeof Message>[]): ModelM
} else {
// New content parts array — collapse to text for LLM
const textSegments: string[] = [];
const attachmentLines: string[] = [];
// Collect attachments into a header block
const attachmentParts = msg.content.filter((p: { type: string }) => p.type === "attachment");
if (attachmentParts.length > 0) {
textSegments.push("User has attached the following files:");
for (const part of attachmentParts) {
const att = (part as { type: string; attachment: { filename: string; mediaType: string; size?: number; path: string } }).attachment;
const sizeStr = att.size ? `, ${formatBytes(att.size)}` : '';
textSegments.push(`- ${att.filename} (${att.mediaType}${sizeStr}) at ${att.path}`);
for (const part of msg.content) {
if (part.type === "attachment") {
const sizeStr = part.size ? `, ${formatBytes(part.size)}` : '';
attachmentLines.push(`- ${part.filename} (${part.mimeType}${sizeStr}) at ${part.path}`);
} else {
textSegments.push(part.text);
}
textSegments.push(""); // blank line separator
}
// Collect text parts
const textParts = msg.content.filter((p: { type: string }) => p.type === "text");
for (const part of textParts) {
textSegments.push((part as { type: string; text: string }).text);
if (attachmentLines.length > 0) {
textSegments.unshift("User has attached the following files:", ...attachmentLines, "");
}
result.push({

View file

@ -49,10 +49,10 @@ export class FSRunsRepo implements IRunsRepo {
let textContent: string | undefined;
if (typeof content === 'string') {
textContent = content;
} else if (Array.isArray(content)) {
} else {
textContent = content
.filter((p: { type: string }) => p.type === 'text')
.map((p: { type: string; text?: string }) => p.text || '')
.filter(p => p.type === 'text')
.map(p => p.text)
.join('');
}
if (textContent && textContent.trim()) {
@ -101,10 +101,10 @@ export class FSRunsRepo implements IRunsRepo {
let textContent: string | undefined;
if (typeof content === 'string') {
textContent = content;
} else if (Array.isArray(content)) {
} else {
textContent = content
.filter((p: { type: string }) => p.type === 'text')
.map((p: { type: string; text?: string }) => p.text || '')
.filter(p => p.type === 'text')
.map(p => p.text)
.join('');
}
if (textContent && textContent.trim()) {
@ -257,13 +257,5 @@ export class FSRunsRepo implements IRunsRepo {
async delete(id: string): Promise<void> {
const filePath = path.join(WorkDir, 'runs', `${id}.jsonl`);
await fsp.unlink(filePath);
// Clean up attachment sidecar directory if it exists
const attachmentsDir = path.join(WorkDir, 'runs', 'attachments', id);
try {
await fsp.rm(attachmentsDir, { recursive: true });
} catch (err: unknown) {
const e = err as { code?: string };
if (e.code !== 'ENOENT') throw err;
}
}
}

View file

@ -28,15 +28,6 @@ export const AssistantContentPart = z.union([
ToolCallPart,
]);
// Metadata about an attached file or image
export const Attachment = z.object({
type: z.enum(["file", "image"]), // extensible — could add "url", "audio" later
path: z.string(), // absolute file path
filename: z.string(), // display name ("photo.png")
mediaType: z.string(), // MIME type ("image/png", "text/plain")
size: z.number().optional(), // bytes
});
// A piece of user-typed text within a content array
export const UserTextPart = z.object({
type: z.literal("text"),
@ -46,7 +37,10 @@ export const UserTextPart = z.object({
// An attachment within a content array
export const UserAttachmentPart = z.object({
type: z.literal("attachment"),
attachment: Attachment,
path: z.string(), // absolute file path
filename: z.string(), // display name ("photo.png")
mimeType: z.string(), // MIME type ("image/png", "text/plain")
size: z.number().optional(), // bytes
});
// Any single part of a user message (text or attachment)