Skip to content

chore(ai): small attachment refactor#841

Merged
ehayes2000 merged 6 commits intomainfrom
attachment-refactor
Jan 8, 2026
Merged

chore(ai): small attachment refactor#841
ehayes2000 merged 6 commits intomainfrom
attachment-refactor

Conversation

@ehayes2000
Copy link
Copy Markdown
Contributor

  • refactor image attachments
  • refactor text attachments
  • arc -> box

Add tighter typing to image attachments.
Add newtype pattern for TextAttachments in preparation of supporting properties.

@ehayes2000 ehayes2000 requested a review from a team as a code owner January 7, 2026 22:38
@gbirman
Copy link
Copy Markdown
Contributor

gbirman commented Jan 7, 2026

pls fix

write!(
f,
"{}",
match self {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not just do "webp" why the match

@ehayes2000 ehayes2000 force-pushed the attachment-refactor branch from 25e2469 to 883df64 Compare January 7, 2026 23:14
{
ImageData::try_from_bytes(bytes.into())
} else {
Err(anyhow::anyhow!("No conversion to image"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

prefer anyhow::bail

{
ImageData::try_from_bytes(bytes.into())
} else {
Err(anyhow::anyhow!("No conversion to image"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same

FileType::Jpeg => Ok(Self::Jpeg),
FileType::Png => Ok(Self::Png),
FileType::Webp => Ok(Self::WebP),
_ => Err(anyhow::anyhow!("No conversion")),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

anyhow::bail

"jpg" => Ok(Self::Jpg),
"jpeg" => Ok(Self::Jpeg),
"png" => Ok(Self::Png),
_ => Err(anyhow::anyhow!("No conversion")),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same

Copy link
Copy Markdown
Contributor

@gbirman gbirman left a comment

Choose a reason for hiding this comment

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

Seems chill, some small nits

"{}",
match self {
Self::WebP => "webp",
Self::Jpg => "Jpg",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are these capitalized but webp isn't?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

all should be lower case good catch

}
}

impl TryFrom<&str> for ImageFormat {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Porque no derive EnumString strum macros? It does this for us

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

don't trust it

use crate::traits::TextAttachment;

#[derive(Debug, Clone, Default)]
pub struct PromptAttachments(Vec<String>);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm assuming these are attachment ids? Is it worth new typing that as well for clarity? If not maybe a comment to clarify what the strings are

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No these aren't id's these are the full text of the attachments. The PromptAttachments struct can only be constructed from structs implementing TextAttachment which formats some fetched attachment for AI.

Base64Image::compress_and_reencode(bytes).map(Self::Bytes)
}

pub(crate) fn dangerously_try_from_string(s: String) -> Result<Self, anyhow::Error> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very dangerous

@ehayes2000 ehayes2000 merged commit 12e939c into main Jan 8, 2026
37 checks passed
@ehayes2000 ehayes2000 deleted the attachment-refactor branch January 8, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants