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

UEX Corp data runner skill #240

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

danielduckworth
Copy link

Hi, I'm working on an additional skill based on vision_ai and uexcorp that captures commodity price data from screenshots and submits them to the /data_submit endpoint.

This is a first pass and not yet tested. I'm just looking for some feedback about the overall structure, flow, and anything I might need to import to start testing it.

@teddybear082
Copy link
Contributor

teddybear082 commented Aug 23, 2024

Neat idea! I don't play star citizen so cannot comment on the substance. I did not even know wingman had a "get skill" function so I need to look into that, very creative. We haven't historically had skills that have relied on other skills but I can see the attractiveness of not having to duplicate a bunch of code that is already there in other skills. At minimum, though, I think during validate there would need to be a check for what happens if the skills it depends on are not there. Have to think about this more. Looking forward to hearing others' thoughts.

EDIT: Ok, subject to further confirmation from Simon/Timo, looking at the code I'm pretty sure there's not actually a get_skill() method in either wingman or openaiwingman, which means this approach would not presently work. But it seems like a good concept worthy of further exploration.

@teddybear082
Copy link
Contributor

There is a self.wingman.skills property / variable though which should be a list, so you might be able to check if skillname in self.wingman.skills

@danielduckworth
Copy link
Author

danielduckworth commented Aug 24, 2024

Thanks for the feedback @teddybear082. I've made some revisions.

  1. The validate() method now checks for the presence of required skills in the self.wingman.skills list.
  2. The capture_and_submit_terminal_data() method now uses next() with a generator expression to find the required skills in the self.wingman.skills list, raising a ValueError if a skill is not found.
  3. The Vision AI prompt has been moved to a separate method for better organization and reusability.
  4. The extract_data_from_screenshot() method now uses the same prompt as defined in the get_vision_ai_prompt() method. It also includes the error handling for the case where the Vision AI skill is not available, consistent with the approach used in the capture_and_submit_terminal_data() method.

@TimoKorinth
Copy link
Contributor

I'm working on an update to the StarHead Skill and wanted to implement something similar as StarHead has also an API to update commodity prices. Maybe we can create one "data runner" skill and custom options to set it to UEXCorp or StarHead (or other APIs in the future)?! What do you think?

@SawPsyder
Copy link
Contributor

I think we should keep them separate as different API's may require different logic and structure. Therefore it might result in many if else statements making the code less readable and harder to maintain.

BTW @danielduckworth - if you need any additions to the uex skill, let me know! (On Discord its JayMatthew)

@TimoKorinth
Copy link
Contributor

Ok, then it's a dogfight!
No, just kidding ;-) But yes, maybe you are right. Then I will try to integrate this into the StarHead skill.

@TimoKorinth
Copy link
Contributor

Maybe it's also best to create a Discord Thread for this?! Makes it easier to communicate. I like the idea of relaying on other skills but right now there is no "analyse_image" function in the vision skill. I would refactor it to easily use the functions, maybe also for making the screenshot etc. So you don't need a dependency to the screenshot skill as well.

@danielduckworth
Copy link
Author

danielduckworth commented Aug 28, 2024

Good idea @TimoKorinth with the Discord thread. @SawPsyder I sent a request on Discord (as dux). In terms of additions to the uexcorp skill, what would be great is a fuzzy search function (or semantic search using embeddings) to match queries (e.g. "Loreville admin terminal") with terminal IDs. Ideally this would be available as a REST API from UEX Corp rather than as a middleware function.

@Shackless
Copy link
Contributor

@TimoKorinth or @SawPsyder would you mind reviewing this PR as I cannot test it? Thank you!

@Shackless
Copy link
Contributor

Shackless commented Sep 16, 2024

@danielduckworth the skill definitely needs an icon in 128x128px (png). How should we credit you as author on https://www.wingman-ai.com/skills ?

…erifying trading terminal data, and submitting verified data to UEXCorp. Use fuzzy search.
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.

5 participants