- Notifications
You must be signed in to change notification settings - Fork144
Refactoring TalksAgent to use active agent#890
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
… view style prompt message template rendering
| 1. First, review the metadata of the video: | ||
| <metadata> | ||
| - title: #{title} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
These interpolations won't work in ERB files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Indeed this needs to be updated and tested still, thanks for pointing this out.
| @@ -0,0 +1,22 @@ | |||
| <%= { | |||
marcorothAug 11, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is there a reason why this is in ERB and in a separate file if it's just a Ruby hash anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Separate file to separate of concerns, but doesn't have to be erb, though I have been using erb to render things like action names dynamically.
| def update_topics | ||
| raw_response = JSON.repair(response.dig("choices", 0, "message", "content")) | ||
| topics = begin | ||
| JSON.parse(raw_response)["topics"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I feel like this should either be directly handled in the framework or maybe the schema needs to be updated?
Isn't the whole idea of structured output that it gives you JSON back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, this also needs testing, I mostly wanted to show how it will look overall compared to how it was before. That way I could get your feedback before spending too much time on it.
adrienpoly commentedAug 11, 2025
Hello thanks for working on this proposal. Just a quick heads up that this part of the code has been working quite well for us up to now and I kind of liked the combination of ActiveJob-performs and associated object we ended up using. At this point of the PR I don't really see a clear benefits of introducing a new dependency/framework. I am following ActiveAgent development I feel like it is made for more complex Agent workflows and we are not there yet our usage is very simple and I think will remain simple for quite some time. If we where to introduce a new framework I would prefer that this happens on some new feature where it really brings something new to the table but happy to be convinced otherwise |
TonsOfFun commentedAug 11, 2025
I appreciate your perspective and the old adage of don't fix what isn't broken certainly applies to your sentiments. I was planning to start with this conversion/refactor as a POC for what you already have as an example of how active agent could be used to make even simple use cases more concise. The ultimate goal would be to continue on to having more advanced agent feature sets for collecting talks, events, and sponsors data. Let me know if you'd like to discuss this proposal/draft PR more or if there is any interest in continuing down this path. |
Refactoring TalksAgent to use active agent's action prompt for action… view style prompt message template rendering