Skip to content

Add plus button to prompt input for file and image attachments#381

Merged
arkml merged 3 commits intodevfrom
feature/prompt-input-file-image-attachments
Feb 26, 2026
Merged

Add plus button to prompt input for file and image attachments#381
arkml merged 3 commits intodevfrom
feature/prompt-input-file-image-attachments

Conversation

@tusharmagar
Copy link
Collaborator

No description provided.

@vercel
Copy link

vercel bot commented Feb 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
rowboat Ready Ready Preview, Comment Feb 26, 2026 4:04pm

Request Review

Copy link
Contributor

@ramnique ramnique left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some concerns to be addressed

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fields type and mediaType are conveying the same information. What I suggest:

  • drop field type
  • rename mediaType to mimeType

Also, we can put these fields directly into UserAttachmentPart , so it becomes:

export const UserAttachmentPart = z.object({
    type: z.literal("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
});

let textContent: string | undefined;
if (typeof content === 'string') {
textContent = content;
} else if (Array.isArray(content)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't need else if here. An else should suffice, because content can either be string or array

textContent = content;
} else if (Array.isArray(content)) {
textContent = content
.filter((p: { type: string }) => p.type === 'text')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't need to type-cast within filter here. typescript ideally shouldn't complain if we do:

 textContent = content
                            .filter(p => p.type === 'text')
                            .map(p => p.text)
                            .join('');

const textSegments: string[] = [];

// Collect attachments into a header block
const attachmentParts = msg.content.filter((p: { type: string }) => p.type === "attachment");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type-casting shouldn't be required here. Also, for readability, we can use a for..of loop here

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this type-casting should not be required here

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need these changes in delete()? AFAIK, attachments are just pointers to existing files on disk

@arkml arkml merged commit cccb7a8 into dev Feb 26, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants