Skip to content
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.

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

Improvement to the ai module #276

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

reflog
Copy link

@reflog reflog commented Mar 15, 2025

Hi @abenz1267

I've 'generalized' the ai module and created another provider - Gemini.
What do you think of this approach?

@abenz1267
Copy link
Owner

abenz1267 commented Mar 15, 2025

Hi,

thanks for working on that. I think the general approach is fine, this PR doesn't function yet though.

Small nitpicks:

  • Anthropic API returned status code 200 => avoid console messages. If you wanna log something, use slog and hide it behind a debug-flag
  • antropic.go => anthropic.go
  • the label should say which model/company is being used, put that info in the sub, where the model is displayed

As for not working: console spits out Anthropic API returned status code 200, but the answer isn't shown.

@reflog
Copy link
Author

reflog commented Mar 17, 2025

@abenz1267 thanks. I've addressed your comments.
The only thing that is not clear to me is the 'sub' usage. When I put model info in it, it doesn't appear in the UI (in the screenshot I have gemini and claude with the same assistant name, but they are indistinguishable from each other)
image

@abenz1267
Copy link
Owner

@abenz1267 thanks. I've addressed your comments. The only thing that is not clear to me is the 'sub' usage. When I put model info in it, it doesn't appear in the UI (in the screenshot I have gemini and claude with the same assistant name, but they are indistinguishable from each other) image

Modules/Plugins have a show_sub_when_single boolean setting that can be set to true. By default the sub won't be shown, when there's only 1 modules, f.e. when doing walker -m ai.

This could be simply changed in the default config.

@reflog
Copy link
Author

reflog commented Mar 17, 2025

@abenz1267 lovely, that works great. I've updated the default on the ai module

Comment on lines +67 to +74
gemini := providers.NewGeminiProvider(ai.config, ai.SpecialFunc)
if gemini != nil {
ai.provider["gemini"] = gemini
}
anthropic := providers.NewAnthropicProvider(ai.config, ai.SpecialFunc)
if anthropic != nil {
ai.provider["anthropic"] = anthropic
}
Copy link
Owner

Choose a reason for hiding this comment

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

This looks rather repetitive to me. It should probably be handled in somekinda loop.

Copy link
Owner

Choose a reason for hiding this comment

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

Also: do you really need the "New*Provider" functions? To me it looks like everything should be handled in the providers SetupData method, since you have a loop calling that already anyways. in ai.SetupData.

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.

2 participants