Feature/images #72

PhantomReactor · 2025-04-25T18:39:27Z

InShot_20250429_212847545.mp4

PhantomReactor · 2025-04-25T18:45:39Z

Tested openai and anthropic. Need to fix test cases and refactor

kujtimiihoxha · 2025-04-25T18:59:58Z

internal/llm/agent/agent.go

@@ -38,7 +39,7 @@ func (e *AgentEvent) Response() message.Message {
 }

 type Service interface {
-	Run(ctx context.Context, sessionID string, content string) (<-chan AgentEvent, error)
+	Run(ctx context.Context, sessionID string, content string, attachments [][]byte) (<-chan AgentEvent, error)


Suggested change

Run(ctx context.Context, sessionID string, content string, attachments [][]byte) (<-chan AgentEvent, error)

Run(ctx context.Context, sessionID string, content string, attachments ...[]byte) (<-chan AgentEvent, error)

Just a small thing maybe this looks better so that you don't have to use nil when you don't have attachments

yeah, you're right. will change

kujtimiihoxha · 2025-04-25T19:00:40Z

@PhantomReactor this is great 🙌

AdityaGadhvi · 2025-04-26T15:17:28Z

keep up the great work!

PhantomReactor · 2025-04-26T17:07:56Z

hey @kujtimiihoxha, somethng's wrong with gemini models or am i just dumb? can't get them working

PhantomReactor · 2025-04-26T17:59:02Z

hey @kujtimiihoxha, somethng's wrong with gemini models or am i just dumb? can't get them working

nevermind, saw your comments in other discussions

kujtimiihoxha · 2025-04-26T17:59:45Z

@PhantomReactor yeah that provides is messed up I did not test it enough, something is definitely wrong with it

PhantomReactor · 2025-04-26T18:06:20Z

@PhantomReactor yeah that provides is messed up I did not test it enough, something is definitely wrong with it

okay. will test it from my end after this

kujtimiihoxha

I know this is not ready but saw a couple of things that maybe can be improved

kujtimiihoxha · 2025-04-25T19:46:12Z

internal/tui/tui.go

@@ -95,6 +100,11 @@ type appModel struct {

 	showInitDialog bool
 	initDialog     dialog.InitDialogCmp
+
+	editingMode bool


I think this came from pulling from main this is removed now.

kujtimiihoxha · 2025-04-27T11:50:16Z

internal/llm/agent/agent.go

@@ -157,7 +156,7 @@ func (a *agent) err(err error) AgentEvent {
 	}
 }

-func (a *agent) Run(ctx context.Context, sessionID string, content string) (<-chan AgentEvent, error) {
+func (a *agent) Run(ctx context.Context, sessionID string, content string, attachmentPaths string, attachments ...[]byte) (<-chan AgentEvent, error) {


can we instead create a type here for attachment something like

type Attachment struct { Path string Data []byte }

And than also update the message.BinaryContent to add the Path attribute, this way we do not need to add a new attribute to the message itself and it keeps the code a bit cleaner?

Didn't push my latest changes, that should address the attachment comment. Will push it when I get back

Any reason we need path in binary content?

Also, I was trying to add image previews to the messages but they just keep aligning diagonally breaking the ui. I will try to fix it but if it takes time let's merge it to master without previews. Will keep you updated.

What is attachmentPaths used for, is it a list of paths for attachments that the message has? I might have misunderstood it, my thought was to store the paths in the binary content so that you don't have to add a new field to the message itself and you know which path represents which data.

I am saving attachment paths in a new db column in messages table. We can use it to fetch all the attachments(assuming the path isn't broken) when retrieving message history and display them on messages on chat page. As I moved path to attachment type I don't see any reason to attach them to binary content. Let me know if I am missing something

My thought was to remove the attachmentPaths from the message and instead add the Path in the BinaryContent instead, this way you have the path and the data (even if the path is broken you have the data the user initially uploaded) and you than you can use myMsg.BinaryContent() to get all binary content including all their paths and you can still display them in the chat page. The main reason to do it this way is to have 1 place we store related image data. Does this explain my thinking better? let me know if I can share more context.

Got it. I had the same thought initially but we will have to create a new table to store binary content. I decided not to considering the length of base64 strings and the size of the encoded attachments.

Hmm what if instead we just update https://github.com/PhantomReactor/opencode/blob/feature/images/internal/message/content.go#L70 to be a string and store the base64 ?

We will be storing all the attachments base64 in db which is not a good idea. Also deserialising them from db would be slow.

Also I understand the downside to storing paths in db is they could be broken if user deletes files but I feel the second one is a better for performance

Yeah I get your concern🤔, I was thinking since this is all local and the DB is per project it would not be a big issue but maybe you are right... in that case maybe we just update the BinaryContent model so that it just stores the Path and nothing else and than you use that to load the images in the history... We might need to just ignore paths that are now broken...

that's right. i am saving it in attachment_paths column in messages table

kujtimiihoxha · 2025-04-27T12:34:46Z

Closes #40

kujtimiihoxha · 2025-04-27T13:19:24Z

I am also adding a new ImageInput flag in the models attributes so you can check that to know if you should include the binary messages or not.

This is in an unrelated PR just wanted to let you know.

kujtimiihoxha · 2025-04-27T18:22:26Z

@PhantomReactor I merged fixes for the gemini provider it should work as expected now.

I am really exited for this PR thanks so much for working on this you are awesome 🙌

…ing rn)

PhantomReactor · 2025-04-27T18:42:10Z

thanks man, i too am excited to work on this. thanks for making this tool

kujtimiihoxha · 2025-04-27T19:00:17Z

@PhantomReactor another thing you could consider is pushing the preview stuff to a follow up pr and just include the paths for now. Another reason that might be a good thing is that rendering the images might make it slow to render if you have many...

PhantomReactor · 2025-04-28T13:10:57Z

It looks like bubbletea doesn't support images. There's an open issue for image support in bubbletea. Images sort of render but the ui just breaks after that. I will just add ASCII previews for file picker and messages will just have paths. Will push the latest changes today.

kujtimiihoxha · 2025-04-30T10:45:02Z

@PhantomReactor this is sooo amazing man, I think a couple of small things.

There is a small issue with the border background not being correct.
I think when you want to navigate we should allow arrow keys also
space or enter should go inside of folders
we should maybe not show the types of files we don't support at all
another thing I don't know is should we default the directory to cwd and allow them to move up if they want to.

PhantomReactor · 2025-04-30T14:16:26Z

Will check 1st and 4th points
I agree with the second and third points. Should be addressed in last commit
I think most users wouldn't have images in their projects.

PhantomReactor added 2 commits April 25, 2025 23:17

added file picker and enabled image processing

4c577f8

pull main branch

5d7c01d

PhantomReactor changed the title ~~Feature/images~~ [WIP] Feature/images Apr 25, 2025

kujtimiihoxha reviewed Apr 25, 2025

View reviewed changes

PhantomReactor added 2 commits April 26, 2025 21:56

display attachment paths in messages page

f9a050a

display delete shortcuts on help

319b799

update filepicker width

1b825d1

kujtimiihoxha requested changes Apr 27, 2025

View reviewed changes

WIP: kitty graphics protocol and ascii image previews (breaks everyth…

39e5739

…ing rn)

PhantomReactor added 5 commits April 29, 2025 10:14

new filepicker and image preview

2ffc4f6

merge main

13b9109

filepicker changes and model validation

d2fd9be

minor change

c94cf17

remove executable

9d0372f

PhantomReactor changed the title ~~[WIP] Feature/images~~ Feature/images Apr 29, 2025

PhantomReactor added 2 commits April 30, 2025 00:10

add attachments when when external editor is used

91839e1

added styles to attachment in messages

1582a81

kujtimiihoxha self-requested a review April 30, 2025 10:49

search, select and navigate with path to file or directory

a3cc47c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/images #72

Feature/images #72

PhantomReactor commented Apr 25, 2025 •

edited

Loading

PhantomReactor commented Apr 25, 2025 •

edited

Loading

kujtimiihoxha Apr 25, 2025

PhantomReactor Apr 26, 2025

kujtimiihoxha commented Apr 25, 2025

AdityaGadhvi commented Apr 26, 2025

PhantomReactor commented Apr 26, 2025

PhantomReactor commented Apr 26, 2025

kujtimiihoxha commented Apr 26, 2025

PhantomReactor commented Apr 26, 2025

kujtimiihoxha left a comment

kujtimiihoxha Apr 25, 2025

kujtimiihoxha Apr 27, 2025

PhantomReactor Apr 27, 2025

kujtimiihoxha Apr 27, 2025

PhantomReactor Apr 27, 2025

kujtimiihoxha Apr 27, 2025

PhantomReactor Apr 27, 2025

kujtimiihoxha Apr 27, 2025

PhantomReactor Apr 27, 2025

kujtimiihoxha Apr 27, 2025

PhantomReactor Apr 27, 2025

kujtimiihoxha commented Apr 27, 2025

kujtimiihoxha commented Apr 27, 2025

kujtimiihoxha commented Apr 27, 2025

PhantomReactor commented Apr 27, 2025

kujtimiihoxha commented Apr 27, 2025

PhantomReactor commented Apr 28, 2025 •

edited

Loading

kujtimiihoxha commented Apr 30, 2025

PhantomReactor commented Apr 30, 2025

	Run(ctx context.Context, sessionID string, content string, attachments [][]byte) (<-chan AgentEvent, error)
	Run(ctx context.Context, sessionID string, content string, attachments ...[]byte) (<-chan AgentEvent, error)

Feature/images #72

Are you sure you want to change the base?

Feature/images #72

Conversation

PhantomReactor commented Apr 25, 2025 • edited Loading

PhantomReactor commented Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kujtimiihoxha commented Apr 25, 2025

AdityaGadhvi commented Apr 26, 2025

PhantomReactor commented Apr 26, 2025

PhantomReactor commented Apr 26, 2025

kujtimiihoxha commented Apr 26, 2025

PhantomReactor commented Apr 26, 2025

kujtimiihoxha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kujtimiihoxha commented Apr 27, 2025

kujtimiihoxha commented Apr 27, 2025

kujtimiihoxha commented Apr 27, 2025

PhantomReactor commented Apr 27, 2025

kujtimiihoxha commented Apr 27, 2025

PhantomReactor commented Apr 28, 2025 • edited Loading

kujtimiihoxha commented Apr 30, 2025

PhantomReactor commented Apr 30, 2025

PhantomReactor commented Apr 25, 2025 •

edited

Loading

PhantomReactor commented Apr 25, 2025 •

edited

Loading

PhantomReactor commented Apr 28, 2025 •

edited

Loading